feat(external-context): Add opt-in auto recall for administrator-owned Mem0 dialects - #11246
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
E2E test reportValidated commit: Environment: macOS, Node.js v24.12.0, npm 10.9.8, local loopback synthetic HTTP provider. Results:
Not validated: a live vendor service or runtime execution on Windows/Linux. Those remain outside the evidence claimed by this PR. |
|
Third pass, same commit. Both earlier passes are on this thread — one at Template looks good ✓ — all nine required headings, in both languages. Problem: still clears the bar, and now with independent corroboration. The feature itself is a capability gap rather than a bug, so no reproduction is owed for it. The fix commit on top is the part that needed evidence, and it has it twice over: the author's own before/after measurements (a stalled TLS handshake exiting clean at ~1783 ms instead of living until the host killed it at ~8 s; a writerless configuration FIFO rejected at ~284 ms instead of blocking ~6.8 s; a 4095-character repeated- Direction: established rather than speculative — this is the fourth increment in a series already merged (skeleton #10149, administrator-owned dialects #10634, publishing the package #10653). The reference agent's changelog supports both halves of the shape: automatic recall is an established direction there ("Claude now automatically records and recalls memories as it works"), and so is refusing a special-file configuration path (the Two direction facts are new to this pass, and I verified both in the tree rather than taking them from the thread. The first cuts against the PR. The second cuts in the PR's favour, and it narrows the risk I have been deferring on. Both the README and the design doc send administrators to the heaviest available mitigation — a separate administrator-controlled What I have not changed my mind about, for the third time: this ships in a published npm package and it makes prompt egress per-turn and automatic. The tarball adds Size: the core-module gate does not apply — nothing under Approach: minimal, and I could not find the 80% cut — the Hook entry, the v3 schema, the two registration examples and the docs are each load-bearing for an administrator to actually install this. Nothing unrelated rode along, and the one structural change (a shared runtime fixture in the integration test) is deduplication the new tests require rather than a drive-by refactor. The one thing I would change is documentation rather than code: name the existing opt-outs above, because "segregate your Risk: no elevated risk signals — none of the 16 changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明第三轮审查，commit 未变。本 thread 上已有前两轮 —— 一轮针对 模板完整 ✓ —— 中英文九个必需小节齐全。 问题： 依然达标，而且现在有了独立佐证。功能本身是能力缺口而非 bug，因此不需要复现。其上的修复提交才是需要证据的部分，而它有两重证据：作者自己的前后实测（TLS 握手停滞时约 1783 ms 干净退出，而不是存活到约 8 秒被宿主杀掉；无写入者的配置 FIFO 在约 284 ms 被拒绝，而不是阻塞约 6.8 秒；4095 字符重复 方向： 是既定的而非臆测的 —— 这是已合并系列的第四个增量（骨架 #10149、管理员维护的 dialect #10634、发布包 #10653）。参考 agent 的 changelog 同时支持这两个形态：自动召回在那边是既定方向（"Claude now automatically records and recalls memories as it works"），拒绝特殊文件配置路径也是（ 本轮有两个新的方向性事实，我都到代码树里核对过，而不是照抄 thread 里的说法。 第一个对本 PR 不利。 第二个对本 PR 有利，而且它收窄了我一直据以暂缓的风险。README 和设计文档都把管理员指向了最重的缓解手段 —— 一个不带该 Hook 的独立受控 第三次重申我没有改变看法的一点：它随已发布的 npm 包一起发布，并且让 prompt 外发变成每轮自动行为。 tarball 给一个以 0.23.0 发布的包新增了 规模： 核心模块门禁不适用 —— 完全没有触及 方案： 最小化，我找不到可以砍掉 80% 的切口 —— Hook 入口、v3 schema、两个注册示例和文档，对管理员真正安装它来说都是必需的。没有夹带无关改动，唯一的结构性改动（集成测试中共享的 runtime fixture）是新测试所需的去重，而不是顺手重构。我唯一会改的是文档而不是代码：把上面那些已存在的退出方式写出来，因为「隔离你的 风险： 无升级风险信号 —— 16 个改动文件中没有一个匹配与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewThird pass on I was wrong last time about which test was fragile, and the right answer is a different problem. My previous pass predicted a Linux-only failure in the stalled-TLS test, because it attaches no What is open is the test nobody would have suspected: the trivial first one. The deterministic flakiness gate in I read the file rather than accepting the label, and the mechanism is a budget inversion, not a race. This matters more than the margin suggests, and it is why I am naming it as the one thing I would want fixed before merge rather than filing it as a nit. It is the exact class the repo's flakiness gate exists to catch: after merge this file runs on every PR that touches the package, and a 1-in-5 timing failure becomes intermittent red that has nothing to do with whoever is being blocked by it. The verify job reported the whole run as not passed for precisely this reason, overriding its own agent verdict of The rest of the production code holds up, and I re-checked the parts that carry weight rather than trusting my earlier notes. The sanitizer rewrite is sound, and the reason is the lookbehind rather than the lookahead: Two findings from the sandboxed report that I confirmed in the code myself, both Suggestions rather than blockers. The first is an attribution error in a comment, and it is load-bearing. The comment above The second is a dead field the PR hardens with a new test. Both shipped examples set Three items carried forward, unchanged, none of them a defect in the production code. One follow-up that belongs in a separate issue, surfaced by this PR but not caused by it: sequenceDiagram
participant P1 as Qwen Code TUI or headless CLI
participant P2 as Hook runner
participant P3 as auto-recall entry
participant P4 as Config loader
participant P5 as Query sanitizer
participant P6 as Request engine
participant P7 as Mem0 provider
P1->>P2: user submits a prompt
P2->>P3: spawn process, event JSON on stdin
P3->>P3: require UserPromptSubmit plus submitted_prompt
P3->>P4: load v3 instance config and dialect
P4-->>P3: reject any path that is not a regular file
P4-->>P3: canonical repository root and credential
P3->>P3: realpath cwd, return empty when outside root
P3->>P5: submitted_prompt only, never expanded prompt
P5-->>P3: cap at 4096 chars, strip secrets, keep 512 code points
P3->>P6: one search with the provider timeout
P6->>P7: bounded POST, no retry, no redirect
P7-->>P6: results, timeout, or transport failure
P6-->>P3: at most five items
P3-->>P2: untrusted_external_context, or empty object on any failure
P3->>P3: flush stdout and exit zero
P2-->>P1: injected as additionalContext
Files changed (all 16 — head unchanged since the prior pass, so this is the full map rather than a delta)
Test evidenceUnattended CI run, so per the triage rules I did not build, run, or execute anything from this PR — no CI on this head is now fully settled and green — nothing is pending, and nothing is red. That is a change from my last pass, which fetched three jobs still running.
The sandboxed What this comment carries: real CI check names and conclusions as fetched, the sandboxed report's measured A/B and mutation results, and my own static verification of the sanitizer complexity, the flush-before-exit ordering, the descriptor cleanup, the budget inversion behind the flake, the dead CI results for
One row per check name (latest run); skipped checks omitted — Sandboxed verification is the lane that matters here, and it has already spoken once — the report above is 中文说明代码审查第三轮审查 上一轮我判断错了哪个测试脆弱，而正确答案是另一个问题。 我之前预测 TLS 停滞测试存在仅 Linux 的失败面，理由是它没有给 Hook 故意抛弃的 socket 挂 真正未决的是没人会怀疑的那个测试：最平凡的第一个。 我读了文件而不是接受那个标签，机制是预算倒挂，不是竞态。 这件事的分量超过那个差值本身，这也是我把它列为「合并前我唯一希望修掉的东西」而不是当成小毛病的原因。它正是仓库抖动门存在要抓的那一类：合并之后这个文件会在每个触及该包的 PR 上运行，而 1/5 的时序失败会变成与被阻塞者毫无关系的间歇性红灯。verify 任务正是因为这一点把整次运行报告为不通过，覆盖了它自己 agent 的 生产代码的其余部分站得住，而我重新核对了承重的部分，没有依赖自己早先的笔记。 脱敏重写是可靠的，原因在于后顾断言而非 lookahead： 沙箱报告里的两个发现我自己在代码中确认过，都是 Suggestion 而非阻塞项。 第一个是注释里的归因错误，而且它是承重的。 第二个是 PR 用新测试加固了一个死字段。两个发布示例都设了 三项沿用，未变，都不是生产代码的缺陷。 一个应该单独提 issue 的后续项，由本 PR 暴露但不是它造成的： （时序图与文件清单见英文部分，内容一致。） 测试证据这是无人值守的 CI 运行，因此按 triage 规则我没有构建、运行或执行本 PR 的任何代码 —— 没有对 PR 树跑 这个 head 上的 CI 现已完全结束并且全绿 —— 没有 pending，也没有红色项。 这与上一轮不同，那次抓取时有三个任务仍在运行。
针对这个 head 的沙箱 本评论携带的证据：抓取时真实的 CI check 名称与结论、沙箱报告实测的 A/B 与变异结果，以及我本人的静态核对 —— 脱敏复杂度、刷完再退出的顺序、描述符清理、抖动背后的预算倒挂、死的 （CI 表格见英文部分的机器可读区域。） 沙箱验证是这里真正重要的通道，而它已经发声过一次 —— 上面的报告就是 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 3/5 — the production code is clean and better tested than I would have managed, the Linux oracle closed my last technical question, and then the sandboxed gate found a real defect that neither CI nor my static reading had: the PR's own new integration test is timing-flaky. Merging still needs a human call I cannot make, and now also needs one small test fix. To be precise about where the score comes from: it is my genuine judgment, not a policy cap. This is a fork PR but the title is Going back to my independent proposal: for this code I would have written what the author wrote. Bound the sanitizer's worst case, stop the process lingering after its output is ready, refuse a configuration path that can block the reader, and make CI execute the artifact that actually ships. I found no simpler path on the first read and none on the third. Where the work exceeds what I would have produced is in the test design, and the sandboxed run is what let me confirm that rather than assume it: five mutants, each a single fix hunk reverted, and zero survivors — every guard is pinned by a test that fails without it, and each kill names real expected-versus-actual values ( What moved since my last pass, in both directions. The technical uncertainty is gone. I was also wrong last time, and it is worth being explicit about that. I predicted a Linux-only failure in the stalled-TLS test over a missing socket error listener. It did not happen — that test passed all five sandboxed rounds and passed on Linux CI, and it turns out to be the only test in the file with an explicit timeout, which is precisely why it was never the exposed one. The real fragility was in the trivial first test, from a budget inversion I only found by reading the spawn helper against the vitest config: two sequential children at 8 s each inside a 5 s envelope. A prediction that missed is still worth having made, because looking for it is what surfaced the file's timing structure — but the finding that matters is the one I did not predict, and it came from a lane I cannot run myself. What did not move is the product question, and it got sharper in one direction and softer in the other. Sharper: I verified in the tree that the repo's own Softer, and this is the part I would most want a maintainer to notice: the mitigation is cheaper than the PR says it is. Both the README and the design doc send administrators to the heaviest available answer — segregate a So the question a maintainer is being asked to accept is narrower than the one I framed twice: one-time administrator opt-in, per-turn automatic egress of sanitized prompt text, with a documented one-flag opt-out for automation that the PR should mention but currently doesn't. Whether that is a trade this project wants to make in a package administrators pin at a version is still not my call, and nothing in the diff, the tests, or the description settles it. Two things stand between this and a merge, and I would handle them in this order. First, the flake — concrete, small, the author's to fix, and the reason the repo's own sandboxed gate reports this head as not passed despite a Am I being a pushover, or worn down by a third pass on the same commit? I checked that directly, because a re-run that ends in the same verdict can mean either "consistently right" or "stopped thinking". The honest answer is that my verdict is stable for a different reason than either prior pass: the first pass could not verify the shipped artifact at all, the second was waiting on a Linux oracle, and this one has both and instead found an observed defect in the test file plus a documentation error that cuts in the PR's favour. That is movement, not stalling. What has not moved across all three is the product question, and it is genuinely not resolvable from what I am allowed to read. So: ⏸️ deferring to a maintainer — not approving, and not requesting changes. Not approving because there is an observed defect on this head and a product decision outstanding; the repo's own verification gate says not-passed, and approving over that would be attesting to something the evidence contradicts. Not requesting changes because nothing in the production code needs rewriting — the flake is a timeout in a test, the documentation gap is a paragraph, and a formal blocking review would misattribute a pending product decision to the quality of the author's work, which is high. I am deliberately leaving no deferred-approval instruction behind. There is no approve-on-green marker in this comment, so the finalize job will update the CI table if anything re-runs but will not approve on my behalf — which is the intended outcome, since my verdict is defer rather than approve-with-a-precondition. For completeness: the PR's own CI is fully settled on this head, so there is nothing left to wait for. The blocker is the verdict, not CI. One housekeeping note so nobody is misled by it: the automated As in both prior passes, I could not resolve an accountable owner deterministically, so I am not @mentioning anyone rather than guessing a login: 中文说明Confidence: 3/5 —— 生产代码是干净的，测试比我自己能做到的更好；Linux 判据闭合了我上一个技术疑问；然后沙箱门找到了一个 CI 和我的静态阅读都没有发现的真实缺陷：PR 自己新增的集成测试存在时序抖动。合并仍然需要一个我做不了的人工判断，而现在还需要一个小的测试修复。 需要说清这个分数的来源：它是我真实的判断，不是策略上限。这虽然是 fork PR，但标题是 回到我自己的独立方案：对这份代码，我会写成作者写的样子。约束脱敏的最坏情况、阻止进程在输出就绪后滞留、拒绝可能阻塞读取方的配置路径、并让 CI 执行真正发布的产物。第一次读时我没找到更简单的路径，第三次读也没有。这份工作超出我会产出水平的地方在测试设计上，而沙箱运行让我得以确认而非假设这一点：五个变异体，每个回退一个修复 hunk，零幸存 —— 每一道防护都被一条「没有它就会失败」的测试钉住，而且每次击杀都给出了真实的 expected 与 actual（ 自上一轮以来移动的部分，两个方向都有。 技术上的不确定性消失了。 我上一轮也判断错了，这一点值得明确说。我预测 TLS 停滞测试会因为缺少 socket error 监听器而只在 Linux 上失败。它没有发生 —— 那个测试在沙箱的五轮里全部通过，在 Linux CI 上也通过，而且它恰恰是文件中唯一带显式超时的测试，这正是它从来不是暴露面的原因。真正的脆弱点在第一个最平凡的测试里，来自一个我只有把 spawn helper 和 vitest 配置对着读才发现的预算倒挂：5 秒的信封里顺序跑两个各 8 秒的子进程。一个没命中的预测仍然值得做出，因为去找它才让这个文件的时序结构浮出水面 —— 但真正重要的发现是我没预测到的那个，而它来自一条我自己无法运行的通道。 没有移动的是产品问题，而它在一个方向上变尖锐、在另一个方向上变缓和。 变尖锐的一面：我到代码树里核实了，仓库自己的 变缓和的一面，也是我最希望 maintainer 注意到的：缓解手段比 PR 所说的便宜。README 和设计文档都把管理员指向最重的答案 —— 隔离一个 所以现在请 maintainer 接受的这个问题，比我两次框定的那个更窄：一次性的管理员启用、每轮自动外发经过脱敏的 prompt 文本，并且存在一个文档应当提及、但目前没提的一键退出方式供自动化使用。这个项目是否愿意在一个管理员按版本固定的包里做这个取舍，仍然不是我能决定的，而 diff、测试和描述三者都不能定论它。 挡在这件事和合并之间的是两件事，我会按这个顺序处理。 第一是抖动 —— 具体、小、由作者修，也是仓库自己的沙箱门把这个 head 报告为不通过的原因，尽管 agent 判定是 我是在当软柿子，还是被同一个 commit 的第三轮磨掉了判断力？我直接检查了这一点，因为一次得出相同结论的重跑，既可能意味着「一贯正确」，也可能意味着「停止思考了」。诚实的回答是：我的结论稳定，但理由与前两轮都不同 —— 第一轮完全无法验证发布产物，第二轮在等一个 Linux 判据，而这一轮两者都有了，取而代之的是在测试文件里发现了一个已观测的缺陷，外加一处对本 PR 有利的文档错误。这是移动，不是停滞。三轮都没有移动的是产品问题，而它确实无法从我被允许阅读的材料里判断。 所以：⏸️ 转交 maintainer —— 不批准，也不要求修改。 不批准，是因为这个 head 上有一个已观测的缺陷和一个未决的产品判断；仓库自己的验证门说的是不通过，越过它批准等于为一个与证据相矛盾的东西背书。不要求修改，是因为生产代码里没有任何需要重写的东西 —— 抖动是测试里的一个超时，文档缺口是一段话，而一次正式的阻塞性评审会把一个悬而未决的产品决定错误地归因于作者工作的质量，而那质量是高的。 我刻意没有留下任何延迟批准的指令。 本评论中没有 approve-on-green 标记，因此 finalize 任务在任何重跑时会更新 CI 表格，但不会代我批准 —— 这正是预期结果，因为我的结论是暂缓，而不是「附带前提条件的批准」。为了完整：这个 head 上 PR 自己的 CI 已全部结束，所以没有任何东西需要等。阻塞项是结论，不是 CI。 一句事务性说明，免得有人被它误导：本 thread 上那条自动的 与前两轮一样，我无法确定性地解析出责任人，因此我不 @ 任何人，而不是猜一个登录名： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
…ation Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Validation follow-up for The three findings from the local review have been addressed. The prompt-origin finding is resolved by documenting the existing protocol: eligible TUI, headless and stream-json user turns can all supply The final isolated loopback E2E run passed all 13 scenarios using the locally built CLI and actual Hook bundle. No vendor service was contacted.
The local CLI continued successfully through TLS failure (3095 ms) and the long-prompt/HTTP-stall case (3401 ms). These whole-CLI timings include startup and model overhead; they are not pure sanitizer or Hook timings. Validation also passed: 88 package tests; root build, bundle, typecheck and lint; package lint; formatting and diff checks; npm pack dry-run with exactly the expected ten files. The package test commands now build and execute the shipped Hook artifact. Independent review found no additional defect and verified regular-file/symlink compatibility for v2/v3 plus redacted v2 failure on instance/dialect FIFOs. Two final self-audit passes were clean. Windows/Linux runtime execution and real vendor services were not tested locally; the new head still needs CI and maintainer review. 中文说明针对 本地审查的三个问题已处理。输入来源问题通过修正文档与现有协议保持一致：符合条件的 TUI、headless 和 stream-json 用户轮次都可能提供 最终使用本地构建的 CLI 和实际 Hook bundle，全部 13 个隔离回环 E2E 场景通过，没有访问任何真实厂商服务。
本地 CLI 在 TLS 失败（3095 ms）和长 prompt/HTTP 停滞（3401 ms）场景均成功继续。这些 CLI 总耗时包含启动与模型开销，不能视作纯脱敏或 Hook 耗时。 其他验证也已通过：88 项包测试；根仓库 build、bundle、typecheck、lint；包级 lint；格式和 diff 检查；npm pack dry-run 恰好包含预期的十个文件。包级测试命令现在会先构建并执行发布用 Hook 产物。独立审查没有发现新缺陷，并验证了 v2/v3 普通文件和符号链接兼容性，以及 v2 在 instance/dialect FIFO 上的脱敏失败输出。最终两轮自审连续 clean。本地未执行 Windows/Linux 或真实厂商服务验证，新 head 仍需 CI 与 maintainer review。 |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — non-deterministic tests (flakiness gate) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 565 passed · 0 failed · 565 total Flakiness gate: ❌ 1 of 4 changed test file(s) returned different results across identical re-runs (5 full round(s)) The deterministic flakiness gate re-ran the test files this PR changes and got different outcomes from identical runs (agent verdict: 中文 — 判定：❌ 不通过 · 测试结果不确定（抖动门）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：565 通过 · 0 失败 · 565 总计 抖动门：❌ 1 of 4 changed test file(s) returned different results across identical re-runs (5 full round(s)) 确定性抖动门将本 PR 改动的测试文件原样重跑了多轮，得到了不一致的结果（agent 判定： Verification reportPR #11246 — deep verificationVerdict: Verified head OID: 中文摘要结论： A/B 结论：中心主张（Hook 始终及时退出并返回 测试有效性：PR 自带套件的控制组 88/88 全绿；5 个单 hunk 变异体全部被击杀，0 幸存者，且每个都由其预期的那条测试以真实的 expected-vs-actual 报错击杀（例如脱敏上限被回退后： Findings（均为 Suggestion）：(1) 二次方复杂度出在 JWT 三段式正则 未覆盖：见 Not covered — 主要是 Windows 运行时、per-commit 归因（depth-2 浅克隆，commit 1 不可达）、真实 TUI/headless 端到端轮次、npm dry-run 包内容。 ScopeCentral claim — the opt-in Secondary claims — (S1) sanitization is bounded, i.e. repeated secret near-misses stay inside the 2 s subprocess deadline; (S2) the default manifest stays MCP-only with exactly Out of scope and listed under Not covered: Windows runtime, live vendors, write paths, per-commit attribution, real TUI turns. Central claim — A/B tableControl construction: Elapsed ms to process exit, real child processes. Provider
Readings, each with its oracle:
Wire oracle on the happy path (both sides asserted): the provider saw exactly 1 request, S1 — sanitization bound: the cap is load-bearing, and the quadratic pattern is not the one the comment namesTwo independent instruments, same conclusion. (a) Per-regex attribution, each pattern applied alone to a 65 536-character hostile input, patterns copied verbatim and asserted present in the shipped bundle (7/7
The quadratic pattern is
Zero hyphens is linear; every nonzero density is quadratic in N. (Correction to my own harness: a fixture I had labelled (b) Growth exponent through the real exported
(c) End-to-end through the full Hook process, real loopback provider, hostile
The 150 000-char row is the sharpest number in this round: the internal 6500 ms wall clock never fired. Scarier consequences that do NOT hold. This is not an unkillable hang and not a remote DoS. Core's deadline timer runs in the parent's event loop, so the child's blocked loop is irrelevant: at Residual cap precision (informational): the guard is S2 — profile separationReal MCP stdio server (
Mutation matrix against the PR's own suite
Survivors: 0 of 5. Every guard the fix commit introduced is pinned by a test that fails without it, and each kill names expected-versus-actual values. The unmutated control is green, so the kills mean something. m4's independent corroboration: the suite measured 3481 ms for One exception noted for completeness, not as a finding: m1's kill message is Corrections
FindingsS1 — Suggestion: the 4096-char cap is the sole defence against a quadratic regex the tests do not name
node tmp/pr11246-verify-20260907-070044/sanitizer-attribution.mjs \
--bundle "$PWD/tmp/pr11246-verify-20260907-070044/variants/m4-no-sanitizer-cap.js" \
--rungs 1024,4096,16384,65536 --rung-timeout 90Consequence if the cap is ever raised or removed by someone trusting the comment: a synchronous spin the internal wall clock cannot interrupt (measured: first stdout byte at 23 920 ms against a 6500 ms timer). Bounded in production by core's group SIGTERM at Minimal suggested fix (not applied, not measured end-to-end)Make the quadratic pattern linear by removing the unbounded backtracking, e.g. require the dots without letting .replace(/\b[A-Za-z0-9_-]{8,}(?:\.[A-Za-z0-9_-]{8,}){2}\b/g, ' ')is the same shape; the effective change is to anchor the run so it cannot be re-scanned from every hyphen boundary — for instance by consuming the whole token once: .replace(/\b(?:[A-Za-z0-9_-]*\.)+[A-Za-z0-9_-]{8,}\b/g, ' ')I did not measure a candidate fix, so treat this as a direction rather than a patch: per this skill's own rule a suggested fix must be driven through the same harnesses and shown to leave benign fixtures byte-identical. What I can state as measured is that the cap is behaviour-preserving on every benign fixture I ran (head vs uncapped identical at ≤4096 chars; S2 — Suggestion:
|
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
Follow-up to the updated Stage 2/3 comments at
The only failed job in the completed CI attempt was Web Shell E2E Smoke. Its Playwright artifact contains 1,537 中文说明Linux CI 已验证当前 head 的 88 项 Mem0 包测试全部通过，其中包含实际 Hook bundle 的集成测试，TLS 握手停滞测试耗时 1621 ms。本轮未复现审查中提到的 socket error 风险；单次通过并不代表已排除所有偶发失败。 普通文件约束有意同时适用于 v2/v3，仍支持指向普通文件的符号链接，兼容性变化已在 PR 正文和验证报告中说明。该变化及显式启用的自动外发范围仍需 maintainer review。单独运行集成测试前需先在包目录执行 本轮唯一失败的 CI job 是 Web Shell smoke。Playwright 重试 trace 中共出现 1,537 次 |
|
Correction (head 更正（head The sandbox verification job has completed. Its report and harness artifacts identify head I checked the two non-blocking suggestions against this head:
The verification report also notes stale pre-existing Hook user documentation; that remains with the previously recorded Core/help documentation follow-up. Qwen Code CI is successful. The separate PR review is still running, and this automated verification verdict is not maintainer approval. 中文说明沙箱验证已完成。报告针对包含当前 head 两条非阻塞建议已对照当前代码核查：JWT 形状正则在取消上限后确有二次复杂度，4096 个码点的上限必须保留，同一事件循环的计时器无法打断同步正则。但报告所称缺少该输入形状的回归测试不成立：已有测试使用 command Hook 示例中的 Qwen Code CI 已成功；独立 PR 审查仍在运行，本次自动验证结论不代表维护者批准。 |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 56 passed · 4 failed · 60 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：56 通过 · 4 失败 · 60 总计 抖动门：✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #11246 — deep verification (follow-up round)Verdict: Verified head OID: This is a follow-up round at an unchanged head. 中文摘要结论： 上一轮结论状态：上一轮 agent 判定 A/B 与复现结论：head/base 两个 commit 对象与上一轮完全相同，合并树逐字节一致。我把门禁用的命令原样在受控 CPU 超订下重跑，二分出了阈值：空载 0/5 失败（最慢测试 173ms，占 5000ms 预算 3%）；4× 超订 0/3（2361ms，47%）；8× 超订 1/4 失败；16× 超订 3/3 与 2/4 失败（ 根因（新发现，本轮最主要证据）：先崩的不是 5000ms 那条，而是四个 爆炸半径： 反直觉的一点：空载余量最紧的是 TLS 测试（1583ms / 5000ms = 32%），但它在 16× 超订下也从未崩溃——因为它的预算是墙钟等待（provider 超时），不是 CPU。真正脆弱的是预算被进程启动吃掉的那几个。按空载余量排序会得出错误结论。 沿用的两条 Suggestion 均已在本 head 重新测量并成立：S1（4096 上限是二次方正则 有效性对照：从源码重建的 bundle 与随包发布的 未覆盖：见 Not covered — 主要是 Windows 运行时、per-commit 归因（depth-2 浅克隆， Previous-finding status
No previous finding was fixed, declined, superseded or worsened — the head did not move. ScopeCentral claim (carried) — the opt-in This round's chosen scope — the head is unchanged and the only thing standing between this PR and a green verify lane is the flakiness gate, which the previous round reported but never diagnosed. So the central claim for this round is:
plus re-measurement of the two carried-forward Suggestions, which are cheap and are the only other open items. Central claim — A/B table (load bisect)The oracle is vitest's own JSON reporter: per-test
The gate's own failing round is the same shape: The two 16× runs failed through different modes. That is itself the diagnosis: this is not one fragile assertion but a file with four independent hard-coded timing budgets, and which one breaks first depends on where the contention lands. Head-side natural durations against each threshold, all measured this round:
The idle-margin column is misleading on its own, and the TLS row is why. It has the tightest margin in the file (32% of budget spent before any contention) and it never broke, even at 16×. Its budget is a wall-clock wait on a deliberately stalled TLS handshake — not CPU — so contention barely touches it. The tests that broke are the ones whose budget is consumed by process startup and parsing a 411 KB bundle, which is exactly the slice contention inflates. Ranking these by idle margin would have pointed at the wrong test. FindingsF1 — Blocking: the new integration file encodes four unmeasured timing budgets; the tightest one measures process startup, not the behaviour it names
const result = spawnSync(process.execPath, ['--input-type=module', '--eval', `
import { createAutoRecallQuery } from ${JSON.stringify(hookBundle.href)};
const query = createAutoRecallQuery(${JSON.stringify(keyword)}.repeat(Math.floor(4096 / ${keyword.length})), '');
process.stdout.write(JSON.stringify(query ?? null));
`], { encoding: 'utf8', timeout: 2000, killSignal: 'SIGKILL' });
expect(result.error).toBeUndefined();The 2000 ms covers node startup plus parsing the 411 KB esbuild bundle plus the sanitizer call. Only the last slice is what the test claims to bound. Decomposed (
At 8× oversubscription, importing the bundle and calling nothing reaches 1965 ms of the 2000 ms deadline. The test's entire margin is consumed before Reproduce: node tmp/pr11246-verify-20260907-090356/flakiness-probe.mjs --rounds 4 --load 512 --label load512
node tmp/pr11246-verify-20260907-090356/deadline-decomposition.mjs 7Why the failure message points at the wrong cause. When the child is SIGKILLed at 2000 ms, Nothing in that message says "node took too long to start". A maintainer triaging it will read it as the sanitizer being slow — the opposite of the truth, since the sanitizer contributed ~0 ms. Ranking this variant by observability: it is the quiet one, because it silently misattributes. Blast radius — this is not confined to the verify lane. Bounded — what does NOT hold. CI defaults to Minimal suggested fix (direction only — NOT applied, NOT measured)Two independent changes, either of which removes the misattribution:
const query = createAutoRecallQuery(...);
const elapsed = performance.now() - t0; // t0 taken AFTER the import
process.stdout.write(JSON.stringify({ query: query ?? null, elapsed }));then
I did not measure either change, so treat both as direction rather than a patch. Per this skill's own rule a suggested fix must be driven through the same harnesses and shown to leave benign behaviour byte-identical; the fixture that would pin change (1) is one asserting the reported F2 — Suggestion (carried, re-measured): the 4096-char cap is the sole defence against a quadratic regex no test namesUnchanged from P2, re-measured at this head with a positive control the previous round's e2e arm lacked. Per-pattern attribution, each pattern applied alone to a 65 536-char input, all seven constructs first asserted present verbatim in the shipped bundle:
Growth exponent through the real exported
End-to-end through the full Hook process against a real v3 runtime and a real loopback provider — the previous round's e2e arm needed this and my first attempt this round did not have it (see Methodology):
The 150 000-char m4 cell is the sharpest number: Consequence is bounded exactly as the previous round established and I did not re-derive: core's deadline timer runs in the parent's loop, so it SIGTERMs the whole group ( New observation, informational. At head the 150 000-char hostile prompt still produced one provider request. The cap truncates to 4096 chars of F3 — Suggestion (carried, re-measured):
|
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Scope: integrations/external-context-mem0/src/ (all source files), schema, package manifest. NOT reviewed: design doc narrative beyond code-describing claims; test fixtures in detail.
Tier: Deep — credential handling, external network calls, user-prompt sanitization.
No blocking findings. No approval blockers.
Checked:
- createAutoRecallQuery (auto-recall.ts) — Array.from slice by code points correct. Credential literal removed first, then SECRET_ASSIGNMENT_PATTERN, Bearer tokens, JWT-shaped 3x8+ segments, 32+ char words. Best-effort as stated.
- FIFO defense (config.ts) — O_RDONLY|O_NONBLOCK prevents blocking open; isFile() rejects FIFOs before reading. O_NONBLOCK??0 Windows fallback correct.
- isWithinRepository — realpath on both sides, relative() with sep, correct for Windows and symlink traversal.
- Timeout/abort (runAutoRecallCli) — single outputStream.write after Promise.race; no double-write; process.exit handles retained handles.
- Type contracts (schemas.ts, types.ts) — parseInstance applies Ajv before type assertion; union type accepted by createRequestEngine.
- Manifest isolation — v2 MCP path unchanged; v3 accepted only by Hook; no automatic enablement.
Not covered: Windows/Linux runtime (macOS only, disclosed); live vendor endpoints.
Cross-check: CI bot ran 3 passes at this commit, no CHANGES_REQUESTED, no findings to confirm/refute.
Reviewed with AI assistance.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Pushed Correction to my previous verification summary: the agent's
Verification on macOS, Node.js v22.22.3:
The production Hook bundle remains byte-identical ( 中文说明已推送 更正此前验证摘要：agent 的
macOS、Node.js v22.22.3 验证结果：相同启动延迟下，原首项测试 5013 ms 超时，修复后 8428 ms 通过，两个真实 child 均在各自 8 秒内以 0 退出、返回 最终集成文件连续五轮 生产 Hook bundle 字节不变（SHA256 同上），TLS/FIFO 性能断言及结果/退出/stderr 检查保留。本次仅改测试和文档，既有范围决定保持不变。没有需解决的行级审查讨论（0/0）。10:10 UTC 状态更新： |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Review pass on head c0fc7ce. No blocking findings. The auto-recall entry point is well-defended for an opt-in hook: admin deployment via managed settings with administrator-owned paths; the repository-root gate realpaths both sides and fails closed (absolute-only, regular-directory, never the filesystem root); the query sanitizer strips code fences, the configured credential literal, secret assignments, Bearer tokens, JWT-shaped strings, and long opaque tokens before anything leaves the machine, capped at 512 chars from a 1 MiB input bound; the 6.5s wall-clock race sits under the 8s hook timeout with stdin destroy; and every failure path fail-opens to {} so a prompt is never blocked or errored by recall. The O_NONBLOCK + regular-file guard on config open (FIFO hang) is a nice touch. Schema v3 keeps v2 callers untouched via the SearchRuntimeConfiguration union. Roughly half the diff is unit/integration tests. review-pr was still running at review time; everything else is green.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- X1-1 design-doc Verification section records an npm pack --dry-run inspection no automation performs (docs/design/external-context-mem0-auto-recall.md:191) — already reported (comment 5564985809)
- X1-2 shipped Hook timeout has no documented floor above the internal 6500 ms budget (examples/managed-auto-recall-user-settings-posix.json:10) — already reported (comment 5564985809)
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI and the package suite ran only on Linux locally, so the shipped Windows Hook example and macOS symlinked-path containment were never executed on their own platforms.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI and the package suite ran only on Linux locally, so the shipped Windows Hook example and macOS symlinked-path containment were never executed on their own platforms.
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| file = await open(path, 'r'); | ||
| // A blocked FIFO open can outlive even the Hook's explicit process exit. | ||
| file = await open(path, constants.O_RDONLY | (constants.O_NONBLOCK ?? 0)); | ||
| if (!(await file.stat()).isFile()) throw new Error('Not a regular file.'); |
There was a problem hiding this comment.
[Suggestion] R1-1: this regular-file guard lands in the readConfigFile shared by both profiles, so it also narrows the pre-existing v2 MCP loader — and no test on either path can kill it.
Two measured consequences. Coverage: a FIFO holding valid configuration with a live writer is rejected today only by this line; delete it and the shipped Hook loads its configuration from the pipe, yet the whole package suite stays green (88/88). The two stalled-FIFO integration cases cannot see it — a writerless FIFO opened O_NONBLOCK EOFs at once, so JSON.parse('') throws into the same catch either way — and the directory case at schemas.test.ts:217-219 is decided by EISDIR either way. So whichever way you resolve this, the decision leaves no trace in the tests. Documentation: an existing v2 deployment feeding its instance JSON through a readable non-regular path — bash process substitution (QWEN_EXTERNAL_CONTEXT_MEM0_CONFIG=/dev/fd/63, measured isFile=false isFIFO=true, content readable), a FIFO, or a character device — loaded fine at the merge base and now fails at startup with the redacted Mem0 extension instance configuration is unavailable. The README troubleshooting row for that exact message (README.md:308) lists only "missing, blank, unresolved, or unreadable", so the administrator debugs a working file instead of the file type. The behaviour change itself is disclosed in your PR body, so what remains actionable is the missing test and the missing v2 documentation.
Witness:
v2 loader, base arm (hunk reverted) vs PR arm:
BASE : {"state":"resolved","schemaVersion":2,"credential":"probe-token"}
PR : {"state":"rejected","message":"Mem0 extension instance configuration is unavailable."}
v3 loader, FIFO WITH a live writer feeding valid config:
INTACT : {"loaded":false,"message":"...instance configuration is unavailable."}
MUTANT : {"loaded":true,"schemaVersion":3,...} <- this line deleted; suite 88/88 green
PROC-SUBST path=/dev/fd/63 isFile=false isFIFO=true readFileSync={"schemaVersion":2}
Add the discriminating fixture — a FIFO with a live writer feeding valid configuration — as a POSIX-only case (it.skipIf(process.platform === 'win32')) asserting both loadAutoRecallRuntimeConfiguration and loadRuntimeConfiguration reject with instance configuration is unavailable, matching the message schemas.test.ts:217-219 already pins for a directory. And document the requirement for both profiles: state it beside "Both files are limited to 64 KiB." in the on-demand section, and name non-regular sources (FIFO, process substitution, device) in the instance configuration is unavailable troubleshooting row.
The fix must not relax the guard: auto-recall.integration.test.ts pins rejects a stalled %s FIFO and exits successfully for both the instance and dialect kinds (exit 0, {}), and config.ts:88 records why — "A blocked FIFO open can outlive even the Hook's explicit process exit." The same readConfigFile serves both profiles via config.ts:52, and MAX_CONFIG_BYTES = 64 * 1024 (config.ts:26) keeps the existing v2 size-bound assertions in force.
The new fed-FIFO case is the acceptance criterion: remove this line and that test must go red — today no test in the package changes behaviour under that mutation on either path.
中文说明
[建议] R1-1：这个「必须是普通文件」的判断位于两个模式共用的 readConfigFile 中，因此它同时收紧了已有的 v2 MCP 加载路径 —— 而且两条路径上都没有测试能够杀掉它。
两个经过实测的后果。覆盖率： 一个持有有效配置、并且有写入者的 FIFO，今天只靠这一行被拒绝；删掉它之后，发布出去的 Hook 会从管道里加载配置，而整个包测试套件仍然全绿（88/88）。两个 stalled-FIFO 集成用例看不到这一点 —— 没有写入者的 FIFO 以 O_NONBLOCK 打开会立刻 EOF，因此 JSON.parse('') 在两种情况下都抛进同一个 catch；而 schemas.test.ts:217-219 的目录用例在两种情况下都由 EISDIR 决定。所以无论你如何处理这一行，测试里都不会留下痕迹。文档： 已有的 v2 部署如果通过可读的非普通文件路径提供 instance JSON —— bash 进程替换（QWEN_EXTERNAL_CONTEXT_MEM0_CONFIG=/dev/fd/63，实测 isFile=false isFIFO=true，内容可读）、FIFO 或字符设备 —— 在 merge base 上可以正常启动，现在会在启动时失败，只给出被脱敏的 Mem0 extension instance configuration is unavailable.。README 中针对这条消息的排障行（README.md:308）只列了 "missing, blank, unresolved, or unreadable"，于是管理员会去排查一个本来正常的文件，而不是文件的类型。行为变更本身已在 PR 描述中披露，因此剩下可执行的是缺失的测试和缺失的 v2 文档。
证据：
v2 加载器，base 分支（回退该 hunk）与 PR 分支：
BASE : {"state":"resolved","schemaVersion":2,"credential":"probe-token"}
PR : {"state":"rejected","message":"Mem0 extension instance configuration is unavailable."}
v3 加载器，带写入者的 FIFO 提供有效配置：
INTACT : {"loaded":false,"message":"...instance configuration is unavailable."}
MUTANT : {"loaded":true,"schemaVersion":3,...} <- 删除该行；套件 88/88 全绿
PROC-SUBST path=/dev/fd/63 isFile=false isFIFO=true readFileSync={"schemaVersion":2}
请补上能够区分的那条 fixture —— 一个带写入者、提供有效配置的 FIFO —— 写成仅 POSIX 的用例（it.skipIf(process.platform === 'win32')），断言 loadAutoRecallRuntimeConfiguration 与 loadRuntimeConfiguration 都以 instance configuration is unavailable 拒绝，与 schemas.test.ts:217-219 已为目录场景钉住的消息一致。同时为两个模式补文档：在按需模式的 "Both files are limited to 64 KiB." 旁边说明该要求，并在 instance configuration is unavailable 排障行中点名非普通文件来源（FIFO、进程替换、设备）。
修复不得放松该判断：auto-recall.integration.test.ts 已为 instance 与 dialect 两种类型钉住 rejects a stalled %s FIFO and exits successfully（退出码 0、输出 {}），且 config.ts:88 记录了原因 —— "A blocked FIFO open can outlive even the Hook's explicit process exit."。同一个 readConfigFile 经 config.ts:52 服务两个模式，MAX_CONFIG_BYTES = 64 * 1024（config.ts:26）也让 schemas.test.ts 中已有的 v2 体积边界断言继续有效。
新增的 fed-FIFO 用例就是验收标准：删除这一行后该测试必须变红 —— 今天在这个包中，没有任何测试会因为该变异而改变行为。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| The Hook requires a non-empty `submitted_prompt` captured before prompt | ||
| expansion. This includes supported interactive TUI submissions and headless | ||
| CLI user turns (`qwen -p` and stream-json input, including SDK clients using | ||
| that path). The field establishes prompt provenance, not a TUI-only origin. |
There was a problem hiding this comment.
[Suggestion] R1-2: this provenance claim is correct per the code, but it contradicts the in-tree hook contract doc, which still says the opposite — and your next paragraph makes the transport question load-bearing.
docs/users/features/hooks.md:624 (untouched by this diff) states "ACP, headless, serve, SDK, and remote-input paths do not produce it in this version", repeated at :669, with the same row in docs/design/submitted-prompt-provenance.md:104 and TUI-only framing at integrations/external-context/README.md:242 and docs/design/direct-external-context-auto-recall.md:122. The same claim is added at docs/design/external-context-mem0-auto-recall.md:109-113. Two shipped docs now answer one data-egress question in opposite directions, and this PR is what makes the answer consequential: the very next paragraph tells administrators to give automation a separate QWEN_HOME because headless turns are eligible.
An administrator deciding whether one shared QWEN_HOME may carry this Hook consults the authoritative hook doc, concludes an automation launcher cannot forward prompt text to the external provider, and skips that isolation. Per the code, every headless user turn does forward its sanitized prompt to the configured third-party provider — nonInteractiveCli.ts:1098 derives submittedPrompt, :2393-2396 passes it on the first UserQuery turn, nonInteractive/session.ts:482 calls runNonInteractive once per stream-json user message, AppContainer.tsx:3086 does the same interactively, and client.ts:3225-3241 gates it only on message type and non-emptiness, with no transport predicate anywhere.
Witness:
witness: not run - the nearest capability was a headless `qwen -p` drive with a
stdin-dumping UserPromptSubmit hook, but packages/core/dist and packages/cli/dist
are absent in this worktree, so it would have needed a full monorepo build.
Ruled instead on the repo's own two green tests, which already pin headless provenance:
nonInteractiveCli.test.ts:1945 plain headless -> submittedPrompt: 'Test input'
nonInteractiveCli.test.ts:6536 stream-json envelope -> submittedPrompt: 'Message from stream-json input'
Reconcile the two docs in this PR or a linked follow-up: drop headless/stream-json from the "do not produce it" list at hooks.md:624 and the parallel displayText sentence at :669, correct the transport row in docs/design/submitted-prompt-provenance.md, and align the TUI-only framing in the sibling integration README and its design doc — or have these new Mem0 docs cite hooks.md as the authority and flag the discrepancy rather than asserting the opposite.
The reconciled wording must not re-assert a TUI-only origin: client.ts:3226-3228 gates the field on messageType === SendMessageType.UserQuery && typeof options?.submittedPrompt === 'string' && options.submittedPrompt.trim().length > 0, with no transport or renderer condition. Only the headless (qwen -p / stream-json) path was verified here — ACP, serve and the SDK path were not — so please do not extend the claim to those without checking them.
中文说明
[建议] R1-2：这条 provenance 说明与代码一致，但它与仓库内的 Hook 契约文档相反 —— 而你紧接着的下一段让「输入通道」这个问题变得有实际后果。
docs/users/features/hooks.md:624（本 diff 未改动）写着 "ACP, headless, serve, SDK, and remote-input paths do not produce it in this version"，在 :669 重复，docs/design/submitted-prompt-provenance.md:104 的表格同样如此，integrations/external-context/README.md:242 与 docs/design/direct-external-context-auto-recall.md:122 也仍是「仅 TUI」的表述。同一条主张又出现在 docs/design/external-context-mem0-auto-recall.md:109-113。于是两份随包发布的文档对同一个数据外发问题给出了相反答案，而正是本 PR 让这个答案有了后果：紧接着的段落告诉管理员要给自动化使用独立的 QWEN_HOME，理由恰恰是 headless 轮次也符合条件。
管理员在判断一个共享 QWEN_HOME 是否可以注册该 Hook 时会去查权威的 Hook 文档，得出「自动化启动器不可能把 prompt 文本发给外部 provider」的结论，于是跳过隔离。而按代码，每个 headless 用户轮次都会把脱敏后的 prompt 发给所配置的第三方 provider —— nonInteractiveCli.ts:1098 派生 submittedPrompt，:2393-2396 在首个 UserQuery 轮次传递，nonInteractive/session.ts:482 对每条 stream-json 用户消息调用一次 runNonInteractive，AppContainer.tsx:3086 在交互路径同理，而 client.ts:3225-3241 只以消息类型与非空为条件，任何地方都没有传输通道判断。
证据：
witness: not run - 最接近的手段是用一个把 stdin 原样打印的 UserPromptSubmit hook
驱动 headless `qwen -p`，但该 worktree 中缺少 packages/core/dist 与 packages/cli/dist，
需要完整的 monorepo 构建。改为依据仓库自身两个已通过的测试，它们已经钉住了 headless provenance：
nonInteractiveCli.test.ts:1945 plain headless -> submittedPrompt: 'Test input'
nonInteractiveCli.test.ts:6536 stream-json envelope -> submittedPrompt: 'Message from stream-json input'
请在本 PR 或一个关联的后续中把两份文档对齐：从 hooks.md:624 的「不会产生该字段」列表以及 :669 的 displayText 平行句中去掉 headless/stream-json，修正 docs/design/submitted-prompt-provenance.md 的传输表格，并对齐同级 integration README 与其设计文档中「仅 TUI」的表述；或者让这些新的 Mem0 文档引用 hooks.md 作为权威并标注分歧，而不是给出相反断言。
对齐后的措辞不得重新断言「仅 TUI 来源」：client.ts:3226-3228 的条件是 messageType === SendMessageType.UserQuery && typeof options?.submittedPrompt === 'string' && options.submittedPrompt.trim().length > 0，其中没有任何传输或渲染条件。本次只验证了 headless（qwen -p / stream-json）路径 —— ACP、serve 与 SDK 路径未验证 —— 因此请勿在未核实前把主张扩展到它们。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| The administrator installs a pinned package version at a stable absolute path, | ||
| creates the v3 instance and dialect files outside ordinary workspaces, injects | ||
| the configuration path and credential through the managed process environment, | ||
| and copies the applicable Hook definition into an administrator-controlled |
There was a problem hiding this comment.
[Suggestion] R1-3: user-scope registration alone is not sufficient, and neither this Deployment section nor the README's steps say so — a workspace-scoped disableAllHooks: true outranks it and recall silently never happens.
disableAllHooks appears in none of the three workspace restriction lists (WORKSPACE_RESTRICTED_SETTINGS is exactly tools.workflowsEnabled, security.allowPrivateNetworkHooks, security.allowedInsecureVoiceBaseUrls, goals.modelProposed — settingsUtils.ts:267-276), and precedence is System Defaults < User < Workspace < System (settings.ts:610-622). So a repository's .qwen/settings.json beats the managed QWEN_HOME/settings.json.
An administrator copies the Hook into the managed user settings; a repository the launcher opens sets disableAllHooks: true. Config.initialize then skips new HookSystem(this) and the MessageBus subscription (config.ts:3156-3164), and client.ts:3207-3223 gates the whole UserPromptSubmit block on hooksEnabled plus a non-null messageBus. No hook process spawns, so nothing even returns {} — the README's "Auto Recall returns {}" troubleshooting row cannot apply — and recall never happens in that repository, with no diagnostic. The same silence occurs under bare or safe mode. The sibling in-tree profile documents and ships exactly this counter-measure: integrations/external-context/README.md step 4 says to point QWEN_CODE_SYSTEM_SETTINGS_PATH at an administrator-controlled copy of examples/managed-auto-recall-system-settings.json, "Its system-level disableAllHooks: false prevents lower-precedence workspace settings from suppressing the required Hook." This package ships only the two user-settings examples, and grep for disableAllHooks or SYSTEM_SETTINGS over its README and this design doc returns zero hits.
Witness:
witness: not run - the nearest capability was a vitest probe on the real settings
merge, but mergeSettings is not exported (settings.ts:588) and settings.ts:11-17
value-imports @qwen-code/qwen-code-core, whose dist/ is absent in this worktree.
Ruled on the quoted precedence order, the three restriction lists, the
config.ts:3156-3164 and :8137-8139 gates, and the sibling package's own shipped
example + documented rationale as in-repo corroboration.
Add the sibling's step here and to the README's Auto Recall steps: point QWEN_CODE_SYSTEM_SETTINGS_PATH at an administrator-controlled settings file carrying disableAllHooks: false, and note that workspace settings, bare mode and safe mode can each suppress the Hook. Optionally ship a matching examples/managed-auto-recall-system-settings.json.
config.ts:8138 is return this.disableAllHooks || this.getBareMode() || this.isSafeMode(); — a system-level disableAllHooks: false cannot re-enable the Hook under bare or safe mode, so the added guidance must not promise recall there.
If you do ship a system-settings example, src/manifest.test.ts should pin it the way its existing it.each case pins the two user-settings examples (asserting disableAllHooks: false and no mcpServers); that assertion is the test that must go red if the example is later dropped or weakened.
中文说明
[建议] R1-3：只在 user 作用域注册并不足够，而本节与 README 的步骤都没有说明这一点 —— 仓库（workspace）作用域的 disableAllHooks: true 优先级更高，会让召回静默地永不发生。
disableAllHooks 不在三个 workspace 限制列表中的任何一个里（WORKSPACE_RESTRICTED_SETTINGS 恰好是 tools.workflowsEnabled、security.allowPrivateNetworkHooks、security.allowedInsecureVoiceBaseUrls、goals.modelProposed —— settingsUtils.ts:267-276），而优先级为 System Defaults < User < Workspace < System（settings.ts:610-622）。因此仓库的 .qwen/settings.json 会压过受管 QWEN_HOME/settings.json。
管理员把 Hook 复制进受管 user settings；启动器打开的某个仓库设置了 disableAllHooks: true。于是 Config.initialize 会跳过 new HookSystem(this) 以及 MessageBus 订阅（config.ts:3156-3164），而 client.ts:3207-3223 把整个 UserPromptSubmit 分支限制在 hooksEnabled 与非空 messageBus 上。根本不会派生 Hook 进程，因此连 {} 都不会返回 —— README 的 "Auto Recall returns {}" 排障行无从适用 —— 该仓库中召回永不发生，也没有任何诊断信息。bare 模式与 safe 模式下同样静默。仓库内的同级方案恰好记录并随包发布了这个对策：integrations/external-context/README.md 第 4 步要求把 QWEN_CODE_SYSTEM_SETTINGS_PATH 指向管理员控制的 examples/managed-auto-recall-system-settings.json 副本，"Its system-level disableAllHooks: false prevents lower-precedence workspace settings from suppressing the required Hook."。本包只发布了两个 user-settings 示例，并且对其 README 与本设计文档 grep disableAllHooks 或 SYSTEM_SETTINGS 均为零命中。
证据：
witness: not run - 最接近的手段是对真实 settings 合并做 vitest 探针，但 mergeSettings
未导出（settings.ts:588），且 settings.ts:11-17 以值方式 import @qwen-code/qwen-code-core，
其 dist/ 在该 worktree 中不存在。改为依据上文引用的优先级顺序、三个限制列表、
config.ts:3156-3164 与 :8137-8139 的门控，以及同级包自身随包发布的示例与文档化理由作为仓库内佐证。
请在本节与 README 的 Auto Recall 步骤中补上同级方案的那一步：把 QWEN_CODE_SYSTEM_SETTINGS_PATH 指向带 disableAllHooks: false 的管理员控制设置文件，并说明 workspace 设置、bare 模式与 safe 模式都可能抑制该 Hook。也可以选择随包发布对应的 examples/managed-auto-recall-system-settings.json。
config.ts:8138 是 return this.disableAllHooks || this.getBareMode() || this.isSafeMode(); —— system 级的 disableAllHooks: false 无法在 bare 或 safe 模式下重新启用 Hook，因此新增说明不得承诺这两种模式下仍能召回。
如果确实随包发布 system-settings 示例，src/manifest.test.ts 应当像现有 it.each 用例钉住两个 user-settings 示例那样钉住它（断言 disableAllHooks: false 且没有 mcpServers）；该断言就是「示例被删除或弱化时必须变红」的测试。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| } | ||
| query = query | ||
| .replace(/(```|~~~)[\s\S]*?\1/g, ' ') | ||
| .replace(/(?:```|~~~)[\s\S]*$/g, ' '); |
There was a problem hiding this comment.
[Suggestion] R1-4: this unterminated-fence branch and the whole ~~~ variant ship with no test — both mutants ran green while leaking the pasted block.
Every fence fixture in the new tests is a closed ``` pair that the preceding regex already consumes (auto-recall.test.ts:38/40 and `:63`), and no fixture in `auto-recall.integration.test.ts` contains a fence at all. An unterminated fence is the ordinary case for a truncated paste — pasting a log or a snippet and submitting before the closing marker.
Both mutants were built into dist/auto-recall.js and run: deleting this line, or dropping ~~~ from the pair, leaves all 88 tests green while the query forwarded to the administrator's external provider changes. None of the four remaining patterns catch the leaked content — no secret keyword, no Bearer, no dotted triplet, no 32+ char token — so the pasted block's internal URLs, headers and paths leave the host. This contradicts the design doc's runtime-flow step 5 ("Remove fenced code") and its Verification claim that sanitization is unit-tested.
Witness:
INTACT : Tests 88 passed (88) | UNTERMINATED => "How do I fix this?"
| TILDE-PAIR => "How do I fix this?"
MUTANT A (delete line 98):
Tests 88 passed (88) | UNTERMINATED => "How do I fix this? ```sh curl
https://internal.corp/api -H 'X-Internal: abc123'"
MUTANT B (drop ~~~ from both fence patterns):
Tests 88 passed (88) | TILDE-PAIR => "How do I fix this? ~~~sh curl
https://internal.corp/api ~~~"
Add cases to the createAutoRecallQuery describe block in src/auto-recall.test.ts, e.g. expect(createAutoRecallQuery('How do I fix this?\n```sh\ncurl https://internal.corp/api\n', '')).toBe('How do I fix this?'), plus the same shape with ~~~ in both open and closed form, so the unterminated branch and the tilde alternative are each pinned.
The first fence regex must survive any consolidation: auto-recall.test.ts:63 asserts createAutoRecallQuery('```text\nonly code\n```', '') is toBeUndefined(), and :47 asserts the terminated-fence case yields exactly 'How should deployment work?'.
The new cases are the acceptance criterion: remove this line (or drop ~~~) and they must go red, while those two existing assertions stay green.
中文说明
[建议] R1-4：这个「未闭合代码围栏」分支以及整个 ~~~ 变体都没有测试就发布了 —— 两个变异体都在泄漏粘贴内容的同时保持全绿。
新测试中的每个围栏 fixture 都是已闭合的 ``` 对，会被前一个正则先消费掉（auto-recall.test.ts:38/40 与 `:63`），而 `auto-recall.integration.test.ts` 中没有任何 fixture 含围栏。未闭合围栏恰恰是「粘贴被截断」的常见形态 —— 用户粘贴日志或代码片段后在闭合标记前就提交。
两个变异体都被构建进 dist/auto-recall.js 并实际运行：删除这一行，或从这对正则中去掉 ~~~，88 个测试全部保持绿色，而发给管理员外部 provider 的 query 发生了变化。剩下四条规则都拦不住泄漏内容 —— 没有密钥关键字、没有 Bearer、没有点分三段、没有 32+ 字符 token —— 因此粘贴块中的内部 URL、请求头与路径会离开主机。这与设计文档运行流程第 5 步（"Remove fenced code"）以及其 Verification 中「脱敏已有单元测试」的说法相矛盾。
证据：
INTACT : Tests 88 passed (88) | UNTERMINATED => "How do I fix this?"
| TILDE-PAIR => "How do I fix this?"
MUTANT A (delete line 98):
Tests 88 passed (88) | UNTERMINATED => "How do I fix this? ```sh curl
https://internal.corp/api -H 'X-Internal: abc123'"
MUTANT B (drop ~~~ from both fence patterns):
Tests 88 passed (88) | TILDE-PAIR => "How do I fix this? ~~~sh curl
https://internal.corp/api ~~~"
请在 src/auto-recall.test.ts 的 createAutoRecallQuery describe 中补用例，例如 expect(createAutoRecallQuery('How do I fix this?\n```sh\ncurl https://internal.corp/api\n', '')).toBe('How do I fix this?')，并补上 ~~~ 的未闭合与已闭合两种形态，使未闭合分支与波浪号分支各自被钉住。
任何合并都必须保留第一个围栏正则：auto-recall.test.ts:63 断言 createAutoRecallQuery('```text\nonly code\n```', '') 为 toBeUndefined()，:47 断言已闭合围栏用例恰好得到 'How should deployment work?'。
新增用例即验收标准：删除这一行（或去掉 ~~~）后它们必须变红，同时上述两条既有断言保持绿色。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| value['hook_event_name'] !== 'UserPromptSubmit' || | ||
| typeof submittedPrompt !== 'string' || | ||
| submittedPrompt.trim().length === 0 || | ||
| typeof cwd !== 'string' |
There was a problem hiding this comment.
[Suggestion] R1-5: this cwd clause is the only thing rejecting a cwd-less event before configuration is loaded, and no test reaches it.
The it.each at auto-recall.test.ts:153 covers {}, a wrong event name, prompt-only, and a whitespace-only submitted_prompt — every one of those still skips after this clause is deleted — and every event in auto-recall.integration.test.ts carries a cwd. So the removal is invisible.
Measured: with the clause gone, a well-formed UserPromptSubmit event with a non-empty submitted_prompt and no cwd proceeds to load the administrator's instance and dialect files from disk (loader calls 0 → 1) and then reaches isWithinRepository('/repository', null). isAbsolute(undefined) throws ERR_INVALID_ARG_TYPE, and config.ts:235-236 calls it outside its try (the try opens at 237), so the TypeError propagates out of runAutoRecall and is absorbed only by the CLI-level .catch(() => ({})). The clean pre-config skip the design's runtime-flow step 2 requires becomes an internal crash path plus a wasted configuration load on every such event — externally still {}, so nothing surfaces.
Witness:
INTACT : PROBE settled {"resolved":{}} | loadConfig calls: 0 | isWithinRepository calls: []
MUTANT : PROBE settled {"resolved":{}} | loadConfig calls: 1 | isWithinRepository calls: [["/repository",null]]
Tests 88 passed (88)
node -e: isAbsolute(undefined) -> TypeError ERR_INVALID_ARG_TYPE
| typeof cwd !== 'string' | |
| typeof cwd !== 'string' |
is what the tests need to reach — add { hook_event_name: 'UserPromptSubmit', submitted_prompt: 'question' } (and optionally a cwd: 42 variant) to the it.each list at src/auto-recall.test.ts:153.
The extended case's existing assertions — loadAutoRecallRuntimeConfiguration not.toHaveBeenCalled() and search not.toHaveBeenCalled() — are the acceptance criterion: they go red when this clause is removed, because the mocked loader would then be invoked for the new input.
中文说明
[建议] R1-5：这个 cwd 判断是「在加载配置之前拒绝没有 cwd 的事件」的唯一依据，而没有任何测试能够到达它。
auto-recall.test.ts:153 的 it.each 覆盖了 {}、错误的事件名、只有 prompt、以及 submitted_prompt 全为空白 —— 删掉本判断后这些用例仍然会跳过 —— 而 auto-recall.integration.test.ts 中每个事件都带 cwd。因此该删除是不可见的。
实测：去掉该判断后，一个格式正确、submitted_prompt 非空但没有 cwd 的 UserPromptSubmit 事件会继续从磁盘加载管理员的 instance 与 dialect 文件（加载调用 0 → 1），随后到达 isWithinRepository('/repository', null)。isAbsolute(undefined) 抛出 ERR_INVALID_ARG_TYPE，而 config.ts:235-236 是在其 try 之外调用它的（try 从 237 行开始），所以该 TypeError 会传播出 runAutoRecall，只被 CLI 层的 .catch(() => ({})) 吞掉。设计文档运行流程第 2 步要求的「加载配置前干净跳过」，于是变成一条内部崩溃路径外加每次事件一次无用的配置加载 —— 对外仍然是 {}，因此什么都不会暴露。
证据：
INTACT : PROBE settled {"resolved":{}} | loadConfig calls: 0 | isWithinRepository calls: []
MUTANT : PROBE settled {"resolved":{}} | loadConfig calls: 1 | isWithinRepository calls: [["/repository",null]]
Tests 88 passed (88)
node -e: isAbsolute(undefined) -> TypeError ERR_INVALID_ARG_TYPE
请在 src/auto-recall.test.ts:153 的 it.each 列表中加入 { hook_event_name: 'UserPromptSubmit', submitted_prompt: 'question' }（可再加一个 cwd: 42 变体），使测试能够到达这一行。
扩展用例已有的断言 —— loadAutoRecallRuntimeConfiguration 的 not.toHaveBeenCalled() 与 search 的 not.toHaveBeenCalled() —— 就是验收标准：删除该判断后它们必须变红，因为模拟的加载器届时会为新输入被调用。
— qwen3.8-max via Qwen Code /review (v0.23.0)
|
|
||
| expect(settings.$version).toBe(4); | ||
| expect(settings.mcpServers).toBeUndefined(); | ||
| expect(Object.keys(settings.hooks ?? {})).toEqual(['UserPromptSubmit']); |
There was a problem hiding this comment.
[Suggestion] R1-8: this test is named "keeps the managed Auto Recall … profile Hook-only" but never pins the fixture's own top-level key set, so a privilege-bearing key added to a shipped example passes green.
Both fixtures under examples/ are the files administrators copy verbatim into the settings.json of an administrator-controlled QWEN_HOME — i.e. user scope, which is exactly the scope that honours privilege-bearing keys. settingsSchema.ts:3229-3238 documents security.allowPrivateNetworkHooks as "Only honored from User, System, and SystemDefaults settings scopes; values set in Workspace settings are ignored so a cloned repository cannot self-grant this bypass", and the CLI's own suite pins that at settings.test.ts:3308. The assertions here check $version, mcpServers and the hook event keys only, so the managed profile whose whole selling point is "Hook-only" can silently start shipping a self-granted network bypass — and the gap is invisible because the assertion the test name promises is the one it does not make. The gap is symmetric: the same body asserts the windows fixture with the same three top-level checks.
Witness:
RUN A "approvalMode": "yolo" added to examples/...-posix.json
v keeps the managed Auto Recall 'posix' profile Hook-only
v keeps the managed Auto Recall 'windows' profile Hook-only
Test Files 1 passed (1) / Tests 4 passed (4)
RUN B "security": {"allowPrivateNetworkHooks": true} + "tools": {"approvalMode": "yolo"}
Test Files 9 passed (9) / Tests 88 passed (88)
| expect(Object.keys(settings.hooks ?? {})).toEqual(['UserPromptSubmit']); | |
| expect(Object.keys(settings.hooks ?? {})).toEqual(['UserPromptSubmit']); | |
| expect(Object.keys(settings).sort()).toEqual(['$version', 'hooks']); |
In both fixtures "hooks" is the first key and "$version": 4 the last (examples/managed-auto-recall-user-settings-posix.json:2 and :19), so Object.keys(settings) yields ['hooks', '$version'] — the .sort() above is what makes the assertion order-independent. Note also that a top-level approvalMode is a legacy v1 key; the canonical v4 location is tools.approvalMode (settingsSchema.ts:2881, mapped by migration/versions/v1-to-v2-shared.ts:67), so use the canonical keys when demonstrating the gap.
The added assertion is its own acceptance criterion: it must go red when a non-hooks/$version key is added to either fixture — both mutations above are green today.
中文说明
[建议] R1-8：这个测试名为 "keeps the managed Auto Recall … profile Hook-only"，却从未钉住 fixture 自身的顶层键集合，因此往随包发布的示例里加一个带权限的键也会全绿通过。
examples/ 下的两个 fixture 正是管理员逐字复制进受管 QWEN_HOME 的 settings.json 的文件 —— 也就是 user 作用域，而这恰恰是会认这些带权限键的作用域。settingsSchema.ts:3229-3238 对 security.allowPrivateNetworkHooks 的说明是 "Only honored from User, System, and SystemDefaults settings scopes; values set in Workspace settings are ignored so a cloned repository cannot self-grant this bypass"，CLI 自身的套件在 settings.test.ts:3308 钉住了该行为。这里的断言只检查 $version、mcpServers 与 Hook 事件键，因此这个以「仅 Hook」为卖点的受管模式可能悄悄开始随包发布一个自授的网络放行 —— 而且这个缺口不可见，因为测试名承诺的那条断言恰恰是它没有做的那条。缺口是对称的：同一个测试体以同样的三条顶层检查断言 windows fixture。
证据：
RUN A "approvalMode": "yolo" added to examples/...-posix.json
v keeps the managed Auto Recall 'posix' profile Hook-only
v keeps the managed Auto Recall 'windows' profile Hook-only
Test Files 1 passed (1) / Tests 4 passed (4)
RUN B "security": {"allowPrivateNetworkHooks": true} + "tools": {"approvalMode": "yolo"}
Test Files 9 passed (9) / Tests 88 passed (88)
（修复代码见上方英文部分的 ```suggestion 块。）
两个 fixture 中 "hooks" 都是第一个键、"$version": 4 是最后一个（examples/managed-auto-recall-user-settings-posix.json:2 与 :19），因此 Object.keys(settings) 得到 ['hooks', '$version'] —— 上面的 .sort() 正是让断言与顺序无关。另请注意，顶层的 approvalMode 是 v1 遗留键，v4 的规范位置是 tools.approvalMode（settingsSchema.ts:2881，由 migration/versions/v1-to-v2-shared.ts:67 映射），所以演示该缺口时请使用规范键。
新增断言本身就是验收标准：当任一 fixture 被加入非 hooks/$version 的键时它必须变红 —— 上面两处变异今天都是绿的。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| [`examples/managed-auto-recall-user-settings-windows.json`](./examples/managed-auto-recall-user-settings-windows.json) | ||
| into the `settings.json` of an administrator-controlled `QWEN_HOME`. Replace | ||
| both command placeholders with fixed absolute Node and `dist/auto-recall.js` | ||
| paths. The Mem0 instance and dialect remain independent JSON files; Qwen |
There was a problem hiding this comment.
[Suggestion] R1-9: this self-contained Auto Recall recipe never names QWEN_EXTERNAL_CONTEXT_MEM0_CONFIG, the only variable that points the Hook at the v3 instance file — and every resulting failure returns {}, so the omission is undetectable from the running system.
The section spans README.md:200-288 and reads as a complete procedure: write the v3 file, npm install --prefix, copy the Hook definition into $QWEN_HOME/settings.json, replace both command placeholders. The variable appears nowhere in it — its only three README occurrences are :166 and the :171 export, both inside step 4 of the on-demand flow exporting the v2 memory.instance.json, and the :309 MCP-profile error row. It also never mentions injecting the variable named by credentialEnv.
An administrator following only this section gets readRequiredEnvironment throwing on the unset value (config.ts:258-267), swallowed by the CLI-level .catch(() => ({})); one who instead exported the v2 path earlier gets parseAutoRecallInstanceConfig rejecting it, because the v3 schema pins schemaVersion const: 3. Either way the Hook fires on every prompt, exits 0 and returns {} forever — indistinguishable from "no matching memories" — while /hooks shows it registered and succeeding and nothing reaches stderr. The Auto Recall returns {} troubleshooting row (:312) lists eight causes and not this one, and the acceptance checklist never mentions it.
Witness:
shipped dist/auto-recall.js built from unmodified source, same hook input in all rows:
A variable UNSET -> exitCode 0, stderr "", stdout {}, provider requests 0
B variable -> v2 instance -> exitCode 0, stderr "", stdout {}, provider requests 0
C variable -> v3 instance -> stdout {"hookSpecificOutput":{..."additionalContext"...}}, requests 1
Add a step here: "Set QWEN_EXTERNAL_CONTEXT_MEM0_CONFIG to the absolute path of this v3 instance file and inject the variable named by its credentialEnv in the launcher environment, as in step 4. The Hook reads the same variable as the on-demand profile; a missing value or a schemaVersion: 2 file makes every invocation return {}." Then add the variable to the Auto Recall returns {} troubleshooting row and an item to the acceptance checklist.
Both profiles read the same variable — const CONFIG_ENV = 'QWEN_EXTERNAL_CONTEXT_MEM0_CONFIG' (src/config.ts:25), consumed by the shared loadConfiguration that both loadRuntimeConfiguration and loadAutoRecallRuntimeConfiguration call — so the added text must not introduce an Auto Recall-specific variable name or imply the two profiles can point at different files in one process.
中文说明
[建议] R1-9：这段自成体系的 Auto Recall 部署步骤从未提到 QWEN_EXTERNAL_CONTEXT_MEM0_CONFIG —— 唯一把 Hook 指向 v3 instance 文件的变量 —— 而所有由此产生的失败都返回 {}，因此从运行中的系统无法察觉这一步被漏掉。
本节覆盖 README.md:200-288，读起来是一套完整流程：编写 v3 文件、npm install --prefix、把 Hook 定义复制进 $QWEN_HOME/settings.json、替换两个命令占位符。该变量在其中一次都没有出现 —— README 中它只有三处：:166 与 :171 的 export（都在按需流程第 4 步里，导出的是 v2 的 memory.instance.json），以及 :309 的 MCP 模式错误行。本节也从未提到要注入 credentialEnv 所指定的变量。
只按本节操作的管理员会遇到 readRequiredEnvironment 因变量未设置而抛错（config.ts:258-267），并被 CLI 层的 .catch(() => ({})) 吞掉；而早先导出过 v2 路径的管理员会遇到 parseAutoRecallInstanceConfig 拒绝该文件，因为 v3 schema 把 schemaVersion 钉为 const: 3。两种情况下 Hook 都会在每个 prompt 上触发、以 0 退出并永远返回 {} —— 与「没有匹配的记忆」完全无法区分 —— 同时 /hooks 显示它已注册且成功，stderr 也没有任何输出。Auto Recall returns {} 排障行（:312）列了八种原因，唯独没有这一种；验收清单也从未提到它。
证据：
shipped dist/auto-recall.js built from unmodified source, same hook input in all rows:
A variable UNSET -> exitCode 0, stderr "", stdout {}, provider requests 0
B variable -> v2 instance -> exitCode 0, stderr "", stdout {}, provider requests 0
C variable -> v3 instance -> stdout {"hookSpecificOutput":{..."additionalContext"...}}, requests 1
请在本节补一步：「在启动器环境中把 QWEN_EXTERNAL_CONTEXT_MEM0_CONFIG 设为该 v3 instance 文件的绝对路径，并像第 4 步那样注入其 credentialEnv 所命名的变量。Hook 读取的是与按需模式相同的变量；值缺失或文件是 schemaVersion: 2 都会让每次调用返回 {}。」随后把该变量加入 Auto Recall returns {} 排障行，并在验收清单中加一条。
两个模式读取的是同一个变量 —— const CONFIG_ENV = 'QWEN_EXTERNAL_CONTEXT_MEM0_CONFIG'（src/config.ts:25），由 loadRuntimeConfiguration 与 loadAutoRecallRuntimeConfiguration 共同调用的 loadConfiguration 消费 —— 因此新增文字不得引入 Auto Recall 专属的变量名，也不得暗示两个模式可以在同一进程中指向不同文件。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| "lint": "eslint src", | ||
| "test": "vitest run --config vitest.config.ts", | ||
| "test:ci": "vitest run --config vitest.config.ts", | ||
| "test": "npm run build && vitest run --config vitest.config.ts", |
There was a problem hiding this comment.
[Suggestion] R1-10: the dist/auto-recall.js prerequisite is enforced only inside these two scripts, so the direct-vitest invocation AGENTS.md calls "always preferred" runs the new integration test against a stale or missing bundle.
npx vitest run src/auto-recall.integration.test.ts never executes the npm run build && prefix, and this package's vitest.config.ts wires no globalSetup guard, unlike packages/cli and packages/core. Measured with the same source mutation, only the rebuild differing: with src mutated and dist left intact, the four bundle-spawning cases all pass green against yesterday's artifact — so the only end-to-end evidence for the shipped Hook reports a sanitizer regression as safe — while after npm run build the same mutation reddens five tests including all four. The missing-dist arm fails with node's own Cannot find module .../dist/auto-recall.js: the artifact path is in the assertion diff, but nothing names npm run build as the fix, which is the confusion scripts/vitest-global-setup.js exists to remove (its header cites issue #9149). Note the build script runs npm run clean first, so a failed esbuild leaves no bundle at all rather than the previous one.
Witness:
src mutated (grep -c '32,' src -> 0), dist intact (-> 1), npx vitest run --config vitest.config.ts:
x src/auto-recall.test.ts > bounds sanitizer work before applying credential patterns
v bounds repeated token near misses in the bundle with a process deadline
v bounds repeated api_key near misses in the bundle with a process deadline
v bounds repeated password near misses in the bundle with a process deadline
v bounds repeated secret near misses in the bundle with a process deadline
Test Files 1 failed | 8 passed (9)
same mutation + npm run build: 5 red, incl. all four bundle cases
expected '"tokentokentoken..."' to be 'null' (auto-recall.integration.test.ts:192)
missing-dist arm: 9 failed (9), "Cannot find module <abs>/dist/auto-recall.js"
Give this package a local guard rather than routing through the shared one: a package-local vitest globalSetup (or an in-file precondition in auto-recall.integration.test.ts) that fails before collection when dist/auto-recall.js is absent or older than the newest src/** input, printing npm run build --workspace=integrations/external-context-mem0 as the fix. A Development-section note in the README is the cheaper alternative.
Do not add this package to the shared guard's DIST_PREREQUISITES: scripts/vitest-global-setup.js:108-131 enumerates only manifest.exports and manifest.main, :162-171 fails loud on an empty enumeration, and this package has neither field — so it would break every test run instead of guarding it. For context, the sibling published package has no build prefix at all (integrations/external-context/package.json "test": "vitest run --config vitest.config.ts") and also spawns built artifacts, so this PR already improves on the sibling; the proportionate fix is package-local.
If you add the local guard, scripts/tests/vitest-global-setup.test.js:364-378 already pins the wiring for packages/cli and packages/core via it.each — adding this package to that list makes it pinned, so deleting the globalSetup line later must turn npm run test:scripts red.
中文说明
[建议] R1-10：dist/auto-recall.js 这个前置条件只在这两个脚本内部被保证，因此 AGENTS.md 称为「始终优先」的直接 vitest 调用会让新的集成测试跑在过期或缺失的 bundle 上。
npx vitest run src/auto-recall.integration.test.ts 从不执行 npm run build && 前缀，而本包的 vitest.config.ts 也没有像 packages/cli 与 packages/core 那样接 globalSetup 守卫。用同一处源码变异、只在是否重新构建上区分，实测结果：src 已变异而 dist 保持原样时，四个派生 bundle 的用例全部绿色通过，跑的是昨天的产物 —— 于是关于发布 Hook 的唯一端到端证据把一次脱敏回归报告为安全 —— 而在 npm run build 之后，同一处变异会让五个测试变红，其中包含全部四个。缺少 dist 的分支会以 node 自己的 Cannot find module .../dist/auto-recall.js 失败：产物路径确实出现在断言差异里，但没有任何地方点名 npm run build 才是修法，而这正是 scripts/vitest-global-setup.js 存在的意义（其文件头引用 issue #9149）。注意 build 脚本会先执行 npm run clean，因此 esbuild 失败会完全没有 bundle，而不是保留上一个。
证据：
src mutated (grep -c '32,' src -> 0), dist intact (-> 1), npx vitest run --config vitest.config.ts:
x src/auto-recall.test.ts > bounds sanitizer work before applying credential patterns
v bounds repeated token near misses in the bundle with a process deadline
v bounds repeated api_key near misses in the bundle with a process deadline
v bounds repeated password near misses in the bundle with a process deadline
v bounds repeated secret near misses in the bundle with a process deadline
Test Files 1 failed | 8 passed (9)
same mutation + npm run build: 5 red, incl. all four bundle cases
expected '"tokentokentoken..."' to be 'null' (auto-recall.integration.test.ts:192)
missing-dist arm: 9 failed (9), "Cannot find module <abs>/dist/auto-recall.js"
请给本包一个本地守卫，而不是走共享守卫：一个包内 vitest globalSetup（或在 auto-recall.integration.test.ts 内做前置检查），在 dist/auto-recall.js 缺失或比最新的 src/** 输入更旧时于收集前失败，并打印 npm run build --workspace=integrations/external-context-mem0 作为修法。更轻的替代是在 README 的开发章节加一句说明。
不要把本包加入共享守卫的 DIST_PREREQUISITES：scripts/vitest-global-setup.js:108-131 只枚举 manifest.exports 与 manifest.main，:162-171 在枚举为空时会大声失败，而本包两个字段都没有 —— 那样会破坏每一次测试运行，而不是守卫它。作为背景，同级的已发布包完全没有 build 前缀（integrations/external-context/package.json 的 "test": "vitest run --config vitest.config.ts"）却同样派生构建产物，所以本 PR 已经比同级更好；相称的修法是包内本地化。
如果你加了本地守卫，scripts/tests/vitest-global-setup.test.js:364-378 已通过 it.each 为 packages/cli 与 packages/core 钉住接线 —— 把本包加入该列表即可钉住，之后删除 globalSetup 那一行必须让 npm run test:scripts 变红。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| return {}; | ||
| } | ||
|
|
||
| const query = createAutoRecallQuery( |
There was a problem hiding this comment.
[Suggestion] R1-11: nothing pins that runAutoRecall passes runtime.credential into createAutoRecallQuery, so the README's "removes … the configured credential" guarantee is unwitnessed at the Hook level.
createAutoRecallQuery is unit-tested with the credential as an explicit second argument, which is exactly why this wiring is invisible: every runAutoRecall case uses prompts that do not contain the fixture credential (auto-recall.test.ts:311), and the integration suite's credential ('runtime-token', auto-recall.integration.test.ts:333) appears only in the asserted authorization header at :117 and in none of its five prompts.
Measured: passing '' here keeps all 88 tests green while the configured credential is forwarded verbatim as the provider search query — a mixed prompt that should sanitize to 'deployment policy' becomes 'deployment policy provider-secret-value', and a credential-only prompt that should return {} with no search at all instead issues a search for the credential itself. The concrete cost: a user who pastes their mem0 token into a prompt — or pastes a log or config error that echoes it — has that token sent as the search query, where the provider persists it as a searchable memory record that later recall turns echo back into model context. That is strictly worse than the transient Authorization header the same endpoint already receives, and it contradicts README.md:282-284 ("the Hook removes fenced code, the configured credential, and common secret shapes").
Witness:
probe driven through runAutoRecall with the fixture credential 'provider-secret-value':
INTACT : mixed prompt -> [[{"query":"deployment policy",...}]]
credential-only -> {"output":{},"searchCalls":[]}
MUTANT (this call passes ''):
mixed prompt -> [[{"query":"deployment policy provider-secret-value",...}]]
credential-only -> {"output":{},"searchCalls":[[{"query":"provider-secret-value",...}]]}
package suite with the mutant: 88 green; tsc --noEmit clean
Add one runAutoRecall case to src/auto-recall.test.ts that puts the fixture credential inside the prompt — submitted_prompt: 'deployment provider-secret-value policy' with search.mockResolvedValue([]) — asserting expect(search).toHaveBeenCalledWith({ query: 'deployment policy', signal: expect.any(AbortSignal) }). Mirror it in auto-recall.integration.test.ts (a prompt containing 'runtime-token', asserting the recorded request body.query does not) to pin it end to end.
auto-recall.ts:69 is if (!query) return {};, so a prompt consisting only of the credential sanitizes to undefined and that variant must expect {} plus expect(search).not.toHaveBeenCalled(), not a search with an empty query; createAutoRecallQuery also strips the credential only when it is non-empty (auto-recall.ts:96). Assert both shapes or the new test will not kill the mutant.
The acceptance criterion: the new case must go red when this call passes '' instead of runtime.credential — today all 88 tests pass under that mutation.
中文说明
[建议] R1-11：没有任何测试钉住 runAutoRecall 会把 runtime.credential 传给 createAutoRecallQuery，因此 README 中「移除……所配置的凭证」这条保证在 Hook 层面没有证据。
createAutoRecallQuery 的单元测试是把凭证作为显式第二个参数传入的，这恰恰让这条接线不可见：所有 runAutoRecall 用例使用的 prompt 都不含 fixture 凭证（auto-recall.test.ts:311），而集成套件的凭证（'runtime-token'，auto-recall.integration.test.ts:333）只出现在 :117 被断言的 authorization 头里，五个 prompt 中一个都没有。
实测：把这里改成传 ''，88 个测试全部保持绿色，而所配置的凭证会被原样当作 provider 的搜索 query 发出 —— 本应脱敏为 'deployment policy' 的混合 prompt 变成 'deployment policy provider-secret-value'，而本应返回 {} 且完全不搜索的「仅凭证」prompt 反而会发起一次以凭证本身为内容的搜索。具体代价：用户把自己的 mem0 token 粘进 prompt —— 或粘贴一段回显了它的日志/配置报错 —— 该 token 就会作为搜索 query 发出，provider 会把它持久化为可检索的记忆条目，并在之后的召回轮次里回灌进模型上下文。这严格劣于同一 endpoint 本来就会收到的短暂 Authorization 头，也与 README.md:282-284（"the Hook removes fenced code, the configured credential, and common secret shapes"）相矛盾。
证据：
probe driven through runAutoRecall with the fixture credential 'provider-secret-value':
INTACT : mixed prompt -> [[{"query":"deployment policy",...}]]
credential-only -> {"output":{},"searchCalls":[]}
MUTANT (this call passes ''):
mixed prompt -> [[{"query":"deployment policy provider-secret-value",...}]]
credential-only -> {"output":{},"searchCalls":[[{"query":"provider-secret-value",...}]]}
package suite with the mutant: 88 green; tsc --noEmit clean
请在 src/auto-recall.test.ts 中加一个 runAutoRecall 用例，把 fixture 凭证放进 prompt —— submitted_prompt: 'deployment provider-secret-value policy'，配合 search.mockResolvedValue([]) —— 断言 expect(search).toHaveBeenCalledWith({ query: 'deployment policy', signal: expect.any(AbortSignal) })。并在 auto-recall.integration.test.ts 中镜像一份（prompt 含 'runtime-token'，断言记录到的请求 body.query 不含它），以端到端钉住。
auto-recall.ts:69 是 if (!query) return {};，因此只由凭证组成的 prompt 会脱敏为 undefined，该变体必须断言 {} 加 expect(search).not.toHaveBeenCalled()，而不是断言一次空 query 的搜索；createAutoRecallQuery 也只在凭证非空时才剥离它（auto-recall.ts:96）。请断言两种形态，否则新测试杀不掉该变异体。
验收标准：当此处传 '' 而非 runtime.credential 时，新用例必须变红 —— 今天在该变异下 88 个测试全部通过。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| import { renderResult } from './profile.js'; | ||
| import { createRequestEngine } from './request-engine.js'; | ||
|
|
||
| const MAX_HOOK_INPUT_BYTES = 1024 * 1024; |
There was a problem hiding this comment.
[Suggestion] R1-12: this bound is counted over the whole hook payload, which is dominated by the prompt field the Hook explicitly never reads — so the effective ceiling on a user's own prompt is about half the documented 1 MiB, and the resulting skip is silent and undocumented.
Core sends both fields with the same text: client.ts:3224 sets prompt and nonInteractiveCli.ts:1098 sets submittedPrompt (interactively, AppContainer.tsx:3086 does the same), and hookEventHandler.ts:163-170 writes them in one object. hookRunner.ts:1295 writes JSON.stringify(input) to the child's stdin with no cap — only hook output is capped, at hookRunner.ts:48.
Measured by binary search against the built bundle: with the duplicated field the largest submitted_prompt that still recalls is 524 160 characters and the first that skips is 524 224; without the duplicate it is 1 048 320. So qwen -p "$(cat build.log)" with a ~600 KiB log makes readHookInput cross the bound and return undefined, and the executable emits {} before configuration is even loaded — even though the two fields the Hook actually consumes are valid and small, and the sanitizer would only ever have used the first 4096 code points. docs/design/external-context-mem0-auto-recall.md:94 says "Read at most 1 MiB of Hook JSON from stdin", which reads as a prompt bound and is not. The skip is silent ({}, empty stderr, no provider request) and appears in neither documented {} list — the design doc's step 8 at :105-106, nor README.md:277-278 — nor in the Auto Recall returns {} troubleshooting row at README.md:312, so an administrator debugging "recall silently stopped on big prompts" has no documented cause.
Witness:
boundary binary-searched against the shipped dist/auto-recall.js as a subprocess
against a local provider, ~12 spawns per arm:
{"duplicate":true, "maxPromptCharsThatRecall":524160, "firstPromptCharsThatSkip":524224,
"atBoundaryPayloadBytes":1048559, "overBoundarySearched":false,
"overBoundaryStdout":"{}", "overBoundaryStderr":""}
{"duplicate":false,"maxPromptCharsThatRecall":1048320,"firstPromptCharsThatSkip":1048384, ...}
Declaration: the stdin payload is a MODEL of core's hook input assembled from the four
core sites above; the CLI was not driven end to end.
The cheap correct fix is documentation: name oversized Hook input in both {} lists and in the troubleshooting row, and state the effective ~512 KiB ceiling on submitted_prompt given that core duplicates the text into prompt. The behavioural alternative is to bound what the Hook consumes rather than the whole payload — raise this constant comfortably above the largest payload core can emit, or read only far enough to extract submitted_prompt and cwd.
docs/design/external-context-mem0-auto-recall.md:94 ("Read at most 1 MiB of Hook JSON from stdin") and the "Bounds and failure semantics" list at :123, which records the other five bounds, must both be updated with any change to this constant.
src/auto-recall.test.ts:229-247 ("accepts exactly 1 MiB and rejects one additional byte before configuration") pins the threshold on both sides and that the oversized case skips loadAutoRecallRuntimeConfiguration; it must be re-pointed at whatever bound you choose, otherwise the constant can drift again with the suite green.
中文说明
[建议] R1-12：这个上限是按整个 Hook payload 计数的，而 payload 的主体恰是 Hook 明确从不读取的 prompt 字段 —— 因此用户自己 prompt 的实际可用上限约为文档所述 1 MiB 的一半，而且由此产生的跳过是静默且未记录的。
core 会以相同文本同时发送两个字段：client.ts:3224 设置 prompt，nonInteractiveCli.ts:1098 设置 submittedPrompt（交互路径由 AppContainer.tsx:3086 同理），hookEventHandler.ts:163-170 把它们写进同一个对象。hookRunner.ts:1295 将 JSON.stringify(input) 写入子进程 stdin 且没有上限 —— 只有 Hook 的输出有上限，见 hookRunner.ts:48。
对构建产物做二分实测：在存在重复字段时，仍能触发召回的最大 submitted_prompt 是 524 160 字符，首个被跳过的是 524 224；不存在重复字段时是 1 048 320。因此 qwen -p "$(cat build.log)" 配合约 600 KiB 的日志会让 readHookInput 越过上限并返回 undefined，可执行文件于是在加载配置之前就输出 {} —— 尽管 Hook 真正消费的两个字段既有效又很小，而脱敏本来也只会用前 4096 个码点。docs/design/external-context-mem0-auto-recall.md:94 写的是 "Read at most 1 MiB of Hook JSON from stdin"，读起来像是 prompt 的上限，但并不是。该跳过是静默的（{}、stderr 为空、无 provider 请求），且两份 {} 清单都没有它 —— 设计文档第 8 步 :105-106、以及 README.md:277-278 —— README.md:312 的 Auto Recall returns {} 排障行也没有，因此管理员在排查「大 prompt 上召回静默失效」时没有任何文档化的原因可循。
证据：
boundary binary-searched against the shipped dist/auto-recall.js as a subprocess
against a local provider, ~12 spawns per arm:
{"duplicate":true, "maxPromptCharsThatRecall":524160, "firstPromptCharsThatSkip":524224,
"atBoundaryPayloadBytes":1048559, "overBoundarySearched":false,
"overBoundaryStdout":"{}", "overBoundaryStderr":""}
{"duplicate":false,"maxPromptCharsThatRecall":1048320,"firstPromptCharsThatSkip":1048384, ...}
Declaration: stdin payload 是依据上述四个 core 位点拼出的 core Hook 输入**模型**；
并未端到端驱动 CLI。
最省事的正确修法是文档：在两份 {} 清单与排障行中点名「Hook 输入超限」，并说明由于 core 会把文本复制进 prompt，submitted_prompt 的实际可用上限约为 512 KiB。行为层面的替代方案是让上限约束 Hook 真正消费的内容而不是整个 payload —— 把该常量提高到远超 core 可能发出的最大 payload，或只读取到足以取出 submitted_prompt 与 cwd 为止。
任何对该常量的修改都必须同时更新 docs/design/external-context-mem0-auto-recall.md:94（"Read at most 1 MiB of Hook JSON from stdin"）与 :123 的 "Bounds and failure semantics" 清单（其中记录了另外五条边界）。
src/auto-recall.test.ts:229-247（"accepts exactly 1 MiB and rejects one additional byte before configuration"）在两侧钉住了阈值，并钉住了超限用例会跳过 loadAutoRecallRuntimeConfiguration；它必须随你选定的新边界一并调整，否则该常量仍可能在套件全绿的情况下再次漂移。
— qwen3.8-max via Qwen Code /review (v0.23.0)
|
⏳ Historical-head review — head moved to 中文说明⏳ 历史 head 评审 —— 本次评审进行中 head 移动到了 |
Local verification on Linux — merge-ready from my sideI built a real environment for this on Linux and drove the shipped artifacts rather than reading them. Verdict: I found nothing blocking at This complements the earlier rounds on this thread instead of repeating them: I did not re-derive the diff or re-run the full mutation matrix. I spent the effort on the one channel that had never been exercised — a real user turn through the real CLI — plus an independent reproduction of the flake, and a base-vs-PR demonstration of the v2 behaviour change. Environment: Debian 13, kernel 6.12.63, 16 vCPU, Node v22.22.2, npm 10.9.7. CLI bundle built from this head ( 1. Deterministic gates at this head
2. The end-to-end turn — the claim no channel had demonstratedCI, the sandboxed run and the static reviews all stopped short of an actual user turn. I ran one, both headless and in a real TUI, with a wire-level oracle on both sides: what leaves for the provider, and what arrives at the model.
Same CLI binary, same prompt, same fixture repository; the only difference is whether the administrator Hook is registered in 26 of 26 scripted checks passed. The ones that carry weight:
S14 is the one I would highlight: it is the difference between "the hook sends the user's typed text" and "the hook sends whatever the CLI expanded that text into", and the second would be a very different privacy story. The expanded prompt does not leave the process. The file path does, as part of the typed text. 3. The flakiness gate finding — reproduced, then measured as fixedI reproduced it deterministically before trusting the label: pin vitest to one CPU with N competing busy loops and run the unchanged file. At
Two things worth adding to what the earlier round found. First, the test that breaks first is not the one that was named: it is the four The fix commit is load-bearing and I measured it under identical contention — same machine, same 28 burners, only the test file swapped:
Separating the startup budget from the execution budget — raising the subprocess deadline to 8000 ms while asserting the sanitizer's own One non-blocking residual, same class, in tests the fix did not touch. At 48 burners the first failure is now 4. The v2 special-file change — demonstrated base vs PR
A v2 instance file delivered through a FIFO with a live writer starts fine on the base build and is rejected on this one ( 5. Sanitizer: the cap is the only bound, and I can show whyMeasured against the shipped bundle:
Worst case at runtime is bounded and I measured that too: against a provider that accepts the socket and never replies, at the schema maximum 6. What actually leaves the process (informational, for the egress decision)The PR says "best effort, not DLP", which is honest. Since the maintainer call here is about egress, here is the concrete shape of it, run through the shipped
Nothing here contradicts the PR — it is what "best effort" means in practice. I am recording it so the decision to enable this in a given deployment is made against measured behaviour rather than an adjective. 7. Repository containment — 14 of 14Root itself, a nested subdirectory, and a symlink whose target is inside the repository are accepted. A sibling directory sharing the root's name prefix, an unrelated directory, an in-repo symlink pointing outside, a relative cwd, a missing cwd, and 8. Independent mutation spot-checkI re-ran three of the five hunks myself rather than take the sandbox's word:
All restored afterwards; 88/88 again. 9. Smaller notes
10. Not validatedWindows and macOS runtime behaviour (the CI matrix skips both for this PR, so there is still no platform signal there); ACP and 中文说明Linux 本地验证 —— 我这边认为可以合入我在 Linux 上搭了一套真实环境，直接驱动发布产物而不是只读代码。结论：在 这一轮是对本 thread 既有轮次的补充，不是重复：我没有重新推导整个 diff，也没有重跑完整的变异矩阵。精力放在此前没有任何通道覆盖过的那一条上 —— 经由真实 CLI 的真实用户轮次 —— 外加对抖动的独立复现，以及 base 与 PR 对照演示 v2 行为变化。 环境： Debian 13，内核 6.12.63，16 vCPU，Node v22.22.2，npm 10.9.7。CLI bundle 由本 head 构建（ 1. 本 head 上的确定性门禁
2. 端到端轮次 —— 此前没有任何通道演示过的那条主张CI、沙箱运行和各次静态评审都止步于真实用户轮次之前。我跑了一次，headless 和真实 TUI 都跑了，并在两侧都做了线级 oracle：什么发给 provider，什么到达模型。 （截图见英文部分。）同一个 CLI 二进制、同一句 prompt、同一个夹具仓库，唯一差别是管理员 Hook 是否注册在 26 项脚本断言全部通过。 其中有分量的几项：
我最想强调的是 S14：它区分的是"hook 发送用户输入的文本"与"hook 发送 CLI 把这段文本展开后的结果"，后者会是完全不同的隐私故事。展开后的 prompt 没有离开进程。文件路径会随输入文本一起出网。 3. 抖动门发现 —— 先复现，再量化修复我没有直接采信这个标签，而是先做了确定性复现：把 vitest 绑到单个 CPU 上，同时在同一 CPU 上跑 N 个忙循环，然后原样运行该文件。 在
相对上一轮的发现，有两点值得补充。第一，最先挂的并不是被点名的那个测试，而是四个 修复 commit 是承重的，我在完全相同的争用下做了对照 —— 同一台机器、同样 28 burners，只替换测试文件（截图见英文部分）。把启动预算与执行预算分开 —— 子进程期限提到 8000 ms，同时在子进程内部断言脱敏自身的 一个非阻塞的残留，同一类问题，在修复未触及的测试里。 48 burners 时最先失败的变成 4. v2 特殊文件行为变化 —— base 与 PR 对照实测
通过带活写入者的 FIFO 提供的 v2 实例文件，在 base 构建上能正常启动，在本 PR 上被拒绝（ 5. 脱敏：cap 是唯一的界限，而且我能给出原因针对发布 bundle 的实测：
运行时最坏情况同样有界，我也量了：面对接受连接但永不响应的 provider、在 schema 最大值 6. 实际会出网的内容（信息项，供 egress 决策参考）PR 写的是"尽力而为，不是 DLP"，这很诚实。既然这里的 maintainer 决策关乎数据出网，我把它的具体形状跑了出来（经由发布的
这些都不与 PR 的说法矛盾 —— 这就是"尽力而为"在实践中的样子。我把它记录下来，是为了让"是否在某个部署里启用"这个决定基于实测行为，而不是一个形容词。 7. 仓库边界约束 —— 14/14仓库根本身、嵌套子目录、目标位于仓库内的符号链接均被接受。共享根名前缀的兄弟目录、无关目录、指向仓库外的仓库内符号链接、相对 cwd、不存在的 cwd 以及 8. 独立变异抽检三个 hunk 我自己重跑了一遍，没有直接采信沙箱结论：
事后全部还原，重新 88/88。 9. 其他小项
10. 未验证Windows 与 macOS 的运行时行为（本 PR 的 CI 矩阵两者都跳过，因此那边仍然没有平台信号）；ACP 与 🤖 Generated with Claude Code — Claude Opus 5 (1M context) |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Incremental review of c0fc7ce (1 commit since 529ac62).
Scope: all 3 changed files in this commit (docs/design, README, auto-recall.integration.test.ts).
No blocking findings.
Checked:
-
Doc updates (design doc + README) — add , , and as explicit automation-disable paths alongside the existing "separate QWEN_HOME" guidance. Accurate description of the options; no behaviour change.
-
Test budget separation () — subprocess raised from 2000 ms to 8000 ms to absorb Node.js startup latency; the correctness assertion moved to on the measured execution time. This correctly decouples process-startup overhead from the sanitizer-deadline guarantee. The test now validates what it claimed to validate without flaking on slow CI runners.
Reviewed with AI assistance.
|
Released in v0.23.1. |
5 participants
What this PR does
Adds an administrator-installed, opt-in
UserPromptSubmitcommand-Hook profile to the published Mem0 External Context package. The profile accepts a strictschemaVersion: 3instance configuration with a canonical repository binding, reuses the administrator-ownedDialectV1and existing bounded request engine, derives its query only fromsubmitted_prompt, and injects at most five retrieved records as structureduntrusted_external_context. The default Extension manifest remains MCP-only with exactlycontext_search, and existingschemaVersion: 2deployments using regular configuration files are unchanged. The package also ships the separate Hook bundle, strict v3 schema, unbranded POSIX and Windows registration examples, and administrator deployment guidance.Why it's needed
Administrators who need deterministic recall before eligible user turns currently must build another integration or rely on the model to choose
context_search. This adds a narrow Hook-only path while keeping endpoints, credentials, fixed scope, repository binding, and enablement outside model control, without changing Qwen Core or shipping provider presets.Reviewer Test Plan
How to verify
DialectV1, and a loopback HTTP provider. Invoke the packaged Hook with a validUserPromptSubmitevent and confirm it sends exactly one bounded request derived fromsubmitted_prompt, not the expanded prompt, then returns the provider result asuntrusted_external_context.{}with exit code zero and no integration-generated stderr after the fixed Hook entry point starts. Include a TCP server that accepts a connection but never completes TLS: the Hook must flush its output and actually exit promptly after the provider timeout. Instance and dialect FIFO paths must be rejected promptly without an outer process kill. Repeatedtoken,api_key,password, andsecretnear misses must take less than 2 seconds of sanitizer work after bundle import, with a separate 8-second subprocess deadline covering startup and exit.context_search; confirm a v3 configuration is rejected by MCP and the default Extension manifest contains no Hook.--bare,--safe-mode, anddisableAllHooks: true) and confirm it reaches the model without executing the Hook or sending a recall request. A separate administrator configuration without this Hook supports automation that needs other Hooks. Events lackingsubmitted_promptmust also send no recall request.Evidence (Before & After)
Before: the package supported only on-demand retrieval through the model-selected
context_searchMCP tool.After: an administrator can explicitly install the separate
UserPromptSubmitcommand Hook so eligible TUI and headless CLI user submissions (including stream-json input from SDK clients) receive bounded, structured, untrusted external context before model invocation.Automated subprocess E2E evidence is posted as a separate PR comment. This change adds no TUI surface, so screenshots are N/A.
Tested on
Environment (optional)
macOS; Node.js v22.22.3; npm 10.9.8; local loopback synthetic HTTP/TCP providers and model; locally built CLI and shipped Hook bundles.
Risk & Scope
submitted_prompt; it is not a TUI-only filter. Automation can disable all Hooks with--bare,--safe-mode, ordisableAllHooks: truein managed settings before startup; the two flags also change which customizations are loaded. When automation needs other Hooks, administrators can use a separate controlledQWEN_HOMEwithout this Hook and omit its configuration and credential from that launcher. Sanitization is best effort rather than DLP, and each eligible turn starts one bounded Node process and may wait for the configured provider timeout.context_searchdeployments remain unchanged; v3 is accepted only by the separately installed Hook and is never enabled automatically.Linked Issues
N/A
中文说明
本 PR 做了什么
为已发布的 Mem0 External Context 包新增一个由管理员安装、显式选择启用的
UserPromptSubmit命令 Hook 模式。该模式接受严格的schemaVersion: 3实例配置并绑定规范化后的仓库目录，复用管理员维护的DialectV1和现有有界请求引擎，只从submitted_prompt派生查询，并以结构化untrusted_external_context的形式注入最多五条检索结果。默认 Extension manifest 仍然只有 MCP，并且只暴露context_search；使用普通配置文件的现有schemaVersion: 2部署保持不变。包内同时发布独立 Hook bundle、严格的 v3 schema、无品牌的 POSIX/Windows 注册示例和管理员部署说明。为什么需要
当前，需要在符合条件的用户轮次前确定性召回上下文的管理员，只能自行构建另一套集成，或者依赖模型主动选择
context_search。本 PR 增加一条范围收敛的 Hook-only 路径，同时继续将 endpoint、凭证、固定 scope、仓库绑定和启用权置于模型控制之外，不修改 Qwen Core，也不发布任何厂商 preset。审阅者测试计划
如何验证
DialectV1和本机回环 HTTP provider。使用有效UserPromptSubmit事件调用打包后的 Hook，确认它只基于submitted_prompt而非扩展后的 prompt 发出恰好一次有界请求，并将 provider 结果作为untrusted_external_context返回。{}、退出码为零，且没有集成自身产生的 stderr。包括用只接受连接、始终不完成 TLS 握手的 TCP 服务验证：Hook 必须刷完输出，并在 provider 超时后及时真正退出。instance 和 dialect 的 FIFO 路径必须被及时拒绝，无需外部终止进程。重复token、api_key、password和secret的近似匹配输入在 bundle 导入完成后的脱敏计算必须低于 2 秒；另有 8 秒子进程期限覆盖启动和退出。context_search；确认 MCP 会拒绝 v3 配置，且默认 Extension manifest 中没有 Hook。--bare、--safe-mode和disableAllHooks: true）运行自动化，确认模型正常调用，但不执行 Hook、不发送召回请求。如果自动化仍需其他 Hook，可使用未注册本 Hook 的独立管理员配置。缺少submitted_prompt的事件也必须不发送召回请求。证据（前后对比）
之前：该包只支持通过模型选择的
context_searchMCP 工具按需检索。之后：管理员可以显式安装独立的
UserPromptSubmit命令 Hook，使符合条件的 TUI 和 headless CLI 用户提交（包括 SDK 客户端的 stream-json 输入）在模型调用前获得有界、结构化且不可信的外部上下文。自动化子进程 E2E 证据会作为独立 PR 评论发布。本改动没有新增 TUI 界面，因此截图为 N/A。
已测试平台
环境（可选）
macOS；Node.js v22.22.3；npm 10.9.8；本机回环 synthetic HTTP/TCP provider 和模型；本地构建的 CLI 与发布用 Hook bundle。
风险与范围
submitted_prompt的 headless 和 stream-json 用户轮次，并非仅限 TUI 的过滤器。自动化可在启动前使用--bare、--safe-mode，或在受控配置中设置disableAllHooks: true来禁用全部 Hook；这两个 flag 还会改变其他自定义配置的加载范围。如果自动化仍需使用其他 Hook，管理员可使用不注册该 Hook 的独立受控QWEN_HOME，并从该启动环境中去掉 Auto Recall 配置和凭证。脱敏是尽力而为的降低风险措施，并非 DLP；每个符合条件的轮次还会启动一个有界 Node 进程，并可能等待配置的 provider 超时。context_search部署保持不变；v3 只由单独安装的 Hook 接受，并且绝不会自动启用。关联 Issue
N/A