Skip to content

feat(external-context): Add daemon memory writes - #11311

Merged
doudouOUC merged 1 commit into
mainfrom
codex/mem0-daemon-write
Sep 7, 2026
Merged

feat(external-context): Add daemon memory writes#11311
doudouOUC merged 1 commit into
mainfrom
codex/mem0-daemon-write

Conversation

@doudouOUC

Copy link
Copy Markdown
Collaborator

What this PR does

Adds an independently enabled memory writer for trusted daemon workspaces. Administrators bind the endpoint, credential and fixed scope through strict V4 configuration and a bounded write dialect. Each approved call sends the exact supplied text once with infer: false, distinguishes stored records from accepted operations and uncertain outcomes, and never automatically retries a write. The default Extension remains search-only, and existing V2 search and V3 Auto Recall configurations retain their meanings.

Web Shell now displays complete literal arguments for generic MCP permissions that lack a dedicated content or diff preview, including when the argument text equals the permission title. The writer uses existing daemon permissions and workspace ownership without changing Core scheduling or adding daemon routes.

Why it's needed

Daemon users need to explicitly save a shared workspace fact and let another session retrieve it. The published configurable Extension only offered retrieval, and generic MCP approval bodies could be hidden even though the daemon already supplied the complete arguments. This provides an opt-in write path whose target and submitted text can be checked before execution.

Reviewer Test Plan

How to verify

  1. Enable the separate writer in a trusted workspace with default approval mode, trust: false and an explicit ask rule. Request a save containing leading/trailing spaces, newlines and Unicode. Confirm that the full literal arguments are readable and that the provider receives no request while approval is pending or after rejection.
  2. Approve a fresh call and confirm exactly one POST containing one user message, the unchanged text, infer: false and the configured scope. A second call must request a new decision under this permission configuration; changing input requires rejecting and issuing a new call.
  3. Use two workspaces with the same MCP server name but different credentials/scopes. Confirm correct ownership through interleaved calls, reload one workspace after its calls finish, and verify only its subsequent writes use the updated configuration. Cancel a pending call and confirm its old vote cannot write.
  4. Drop the provider response after accepting the request, or let its response body time out. Confirm an unknown result and one provider request. Terminate the MCP transport after submission and confirm Core skips unsafe replay.
  5. Configure the existing reader against the same corpus. Save through one client, then query through a different client in a new session without including the answer in the query. Confirm the returned fact reaches the model context and another workspace's scope does not return it.

Evidence (Before & After)

Before: an isolated global qwen 0.23.0 daemon emitted complete raw arguments with empty content and a compact JSON title. Passing that actual event through the baseline source adapter produced body text equal to the title, which the component's display condition hid. This baseline combines a real daemon event and source adapter observation; it is not a before-browser screenshot.

After: actual Chrome interaction with the local bundle showed a 3,046-code-point body whose displayed JSON decoded exactly to the pending arguments, including whitespace and Unicode. Format controls were visibly escaped and the final marker remained visible after scrolling. Browser approval produced one stored result and one exact provider request; a second call displayed a new card, and browser rejection left the request count at one. Browser voters differed from the script client that initiated the prompts.

Local verification passed: root build, typecheck and bundle; 182 Mem0 package tests; 67 related Web Shell tests; one repository daemon integration test; 18 additional real-daemon scenarios against synthetic services; changed-file lint/format checks; and an actual package archive started outside the repository and node_modules. Two self-audit passes and an independent read-only review found no actionable defects. The separate E2E report comment records scenario results and limitations.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

macOS 26.6.2, Node v22.22.3, npm 10.9.8, locally built daemon bundle, real ACP/stdio MCP processes, loopback synthetic model and memory services, and actual Chrome Web Shell. Test HOME, configuration and runtime directories were isolated. Verification used the implementation on main 63578c7eb3; the PR is rebased onto the subsequent CI-only main update without changing the verified production or test files.

Risk & Scope

  • Main risk or tradeoff: a provider may commit before a timeout or disconnect, so uncertain results never imply rollback and must not be retried automatically. Human approval depends on existing policy; YOLO and approving PermissionRequest Hooks retain their existing behavior. The generic argument preview also affects other MCP tools without dedicated previews.
  • Not validated / out of scope: real Holo conformance stopped at a preflight list returning HTTP 403, with zero creates; exact infer: false storage, preservation of existing records and the real-service cross-session recall loop remain unverified. ACP-over-HTTP disconnects, standalone trust revocation and MCP restart while approval is pending were not exercised. Automatic writing, deletion, CLI/TUI onboarding, Conversations and cross-workspace cwd migration are outside this first daemon profile. Local synthetic success is not a service-compatibility claim.
  • Breaking changes / migration notes: no existing configuration migration or default enablement. Administrators explicitly add the writer and its V4 configuration per workspace. Fixed scope and cwd are routing constraints, not a tenant authorization boundary; trusted clients can still override MCP configuration under the existing daemon trust model.

Linked Issues

Related to #9951. This implements its explicit-write requirement through the current configurable Extension architecture, rather than the issue's older built-in provider proposal; it does not close that broader issue.

中文说明

此 PR 的内容

为可信 daemon workspace 增加单独启用的记忆写入入口。管理员通过严格的 V4 配置和有限写入 dialect 绑定服务地址、凭证与固定 scope。每次获准调用以 infer: false 原样提交正文一次,区分已保存记录、已受理操作与不确定结果,并且不会自动重试写入。默认 Extension 仍只提供搜索,现有 V2 搜索和 V3 Auto Recall 配置含义保持不变。

对于没有专用正文或 diff 预览的通用 MCP 审批,Web Shell 现在展示完整的字面参数,即使参数文本与审批标题相同也能显示。writer 复用现有 daemon 权限和 workspace 归属,不修改 Core 调度,也不新增 daemon 路由。

为什么需要

daemon 用户需要明确保存一条共享 workspace 事实,并让另一个会话召回。已发布的可配置 Extension 只有读取能力;虽然 daemon 已携带完整参数,通用 MCP 审批正文仍可能被隐藏。本次提供显式启用的写入路径,让执行前能够检查目标和拟提交正文。

审阅者测试计划

如何验证

  1. 在可信 workspace 中单独启用 writer,采用默认审批模式、trust: false 和显式 ask 规则。请求保存含首尾空白、换行和 Unicode 的正文。确认完整字面参数可阅读,且待审批及拒绝后 provider 均未收到请求。
  2. 批准一个新调用,确认恰好发送一次 POST,包含一条 user message、未改写的正文、infer: false 和配置的 scope。在该权限配置下,第二次调用必须请求新的决定;修改正文需要先拒绝,再重新发起调用。
  3. 为两个 workspace 使用相同 MCP server 名、不同凭证和 scope。交错调用时确认归属正确;等待其中一个 workspace 的调用结束后重载其配置,确认只有它之后的写入使用新配置。取消待审批调用,确认旧审批票无法触发写入。
  4. provider 接收请求后丢弃响应,或让响应正文读取超时。确认返回 unknown 且仅有一次 provider 请求。提交后终止 MCP transport,确认 Core 跳过不安全重放。
  5. 将现有 reader 配置到相同记忆库。通过一个客户端保存,再由另一个客户端的新会话查询,查询中不包含答案。确认返回事实进入模型上下文,另一个 workspace 的 scope 不会返回该事实。

证据(修改前后)

修改前:隔离的全局 qwen 0.23.0 daemon 发出了完整原始参数,content 为空,title 是紧凑 JSON。将该真实事件传入基线源码 adapter 后,正文文本与标题相同,因此被组件显示条件隐藏。该基线结合了真实 daemon 事件和源码 adapter 观察,并非修改前的浏览器截图。

修改后:通过实际 Chrome 操作本地 bundle,展示了 3,046 个 code point 的正文;显示的 JSON 解码后与待审批参数完全相同,包括空白和 Unicode。格式控制字符明确转义,滚动后仍能看到末尾标记。浏览器批准后产生一个 stored 结果和一次精确的 provider 请求;第二次调用出现新卡片,浏览器拒绝后请求总数仍为一。浏览器投票客户端与发起 prompt 的脚本客户端不同。

本地验证通过:全仓 build、typecheck 和 bundle;Mem0 包 182 项测试;Web Shell 相关 67 项测试;仓库 daemon 集成测试 1 项;合成服务上的额外 18 组真实 daemon 场景;修改文件的 lint/format 检查;以及将实际打包产物放在仓库和 node_modules 之外启动。两轮自审和一次独立只读审查均未发现可操作缺陷。单独的 E2E 报告评论记录了场景结果和限制。

测试平台

OS 状态
🍏 macOS ✅ 已测试
🪟 Windows ⚠️ 未测试
🐧 Linux ⚠️ 未测试

环境(可选)

macOS 26.6.2、Node v22.22.3、npm 10.9.8、本地构建的 daemon bundle、真实 ACP/stdio MCP 进程、loopback 合成模型和记忆服务,以及实际 Chrome Web Shell。测试 HOME、配置和 runtime 目录均已隔离。验证基于 main 63578c7eb3 上的实现;PR 已 rebase 到随后仅修改 CI 的 main 更新,已验证的生产代码和测试文件没有改变。

风险与范围

  • 主要风险或取舍:provider 可能在超时或断线之前完成提交,因此不确定结果不代表回滚,也不能自动重试。人工审批取决于现有策略;YOLO 和批准调用的 PermissionRequest Hook 保留原有行为。通用参数预览也影响没有专用预览的其他 MCP 工具。
  • 未验证 / 范围之外:真实 Holo conformance 在前置 list 返回 HTTP 403 后停止,create 为零;infer: false 原样保存、不改写已有记录及真实服务的跨会话召回闭环尚未验证。未执行 ACP-over-HTTP 断线、单独撤销 trust 和待审批时重启 MCP 的测试。自动写入、删除、CLI/TUI 接入、Conversations 和跨 workspace cwd 迁移不属于首版 daemon 范围。本地合成测试成功不代表服务兼容已通过。
  • 破坏性变更 / 迁移说明:现有配置无需迁移,也不会默认启用 writer。管理员需为每个 workspace 显式添加 writer 和 V4 配置。固定 scope 和 cwd 是路由约束,不是租户鉴权边界;可信客户端仍能按现有 daemon 信任模型覆盖 MCP 配置。

关联 Issue

关联 #9951。本次通过当前可配置 Extension 架构实现其中的显式写入需求,采用的并非该 issue 较早提出的内置 provider 方案,因此不关闭这个更大范围的 issue。

Add an opt-in workspace-bound writer and complete literal MCP approval previews.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

E2E verification report

Result: local synthetic-service and actual-browser verification passed. Real Holo conformance remains blocked by HTTP 403 before any create.

Reviewed PR commit: d489e295f5697e0dce5252f4049736afe6b6cc4d. Verification ran on the implementation based on main 63578c7eb3; the branch was subsequently rebased onto the CI-only update a1cbe75cf1. All verified production and test file hashes were checked again after rebase and remained unchanged. The only preparation edit removed local historical evidence paths from the design document.

Environment: macOS 26.6.2, Node v22.22.3, npm 10.9.8, local daemon bundle, real ACP/stdio MCP, loopback synthetic model and memory services, and actual Chrome. Separate HOME, configuration and runtime directories isolated the fixtures from existing user daemons. Windows and Linux were not run locally.

Reproducible repository checks

Verification Result
Root build / typecheck / bundle PASS
Mem0 package suite, including V2/V3 regressions and packaged writer stdio-to-HTTP 182/182 PASS
Web Shell adapter and ToolApproval regressions 67/67 PASS
Repository daemon integration test 1/1 PASS
Changed-file ESLint / Prettier and diff whitespace PASS
Actual npm archive extracted outside the repository and node_modules Starts independently, only context_remember, empty stderr, zero provider requests
Two self-audit passes and independent read-only review No actionable findings

The committed daemon regression is external-context-mem0-daemon-write.test.ts. After building and bundling, run it from integration-tests with QWEN_SANDBOX=false npx vitest run cli/external-context-mem0-daemon-write.test.ts --retry=0. The package and Web Shell suites run from their respective package directories.

Additional real-daemon verification

All 18 scenarios passed through real daemon / ACP / MCP processes with synthetic services. Together they intentionally issued 15 provider writes and two reader queries; a request count is not a claim of real-service persistence.

Scenario Observed result
Default mode with explicit ask; pending then reject Complete raw input; zero writes
Approve exact input, then issue a second call One request per approval; second call requires a new vote
Cross-session vote and invalid option No write
Cancel pending call, then send its old vote No write
Disconnect all REST SSE subscriptions, reconnect and approve Remains pending with zero writes, then writes once
Provider accepts request then drops HTTP unknown, one request, no replay
Provider accepts request then exceeds writer timeout unknown, one request, no replay
AUTO with explicit ask Waits for human approval
YOLO with ordinary ask Executes under existing bypass policy
Ordinary allow without ask Executes under existing allow policy
PermissionRequest Hook approves and supplies updatedInput Uses Hook-modified content without a client vote, as existing policy specifies
A/B workspaces share an MCP server name but have different bindings Interleaved calls retain the correct credentials, scope and result
Workspace C omits writer configuration Model tool list contains no writer
Alice saves; Bob uses another client and new session Answer-free query retrieves the fact into model context; B scope does not
Restart B after changing its config at the same path B uses new scope; A remains unchanged
Kill MCP process after the provider receives its request Core reports “Automatic replay was skipped”; one request
Remove a runtime with pending approval and re-register its path Old vote cannot write in the new generation; a fresh approved call works
Positive permission response timeout Approval expires without a provider request

Client permission replies carrying updatedInput did not change the originally approved arguments. This differs from an approving PermissionRequest Hook, which intentionally retains the existing ability to rewrite input before execution.

The deterministic model recorded the actual requests and tool results. The Alice/Bob check proves that the retrieved fact reached the model context; it does not evaluate natural-language answer quality.

Actual Chrome approval evidence

The baseline used a real global daemon permission event and the source adapter: rawInput was complete, content was empty, and the fallback body equalled the compact JSON title, activating the old hiding condition. No before-browser screenshot was captured.

After the change, the actual DOM JSON decoded exactly to the pending arguments: 3,046 content code points / 3,334 rendered JSON characters. Leading and trailing spaces, newlines, Chinese, emoji and quotes were preserved; a format control appeared as a visible Unicode escape. The body was scrolled to its final marker (scrollTop 307, clientHeight 300, scrollHeight 607).

Browser Yes, allow once produced stored and one exact provider request. A fresh call showed another card; browser Reject displayed cancellation and left the total at one request. Both permission resolutions identified the browser's client, distinct from the script client that initiated the prompts.

Limits and cleanup

The real Holo preflight list against a fresh isolated scope returned HTTP 403 in 188 ms. Create count was zero, no record needed deletion, and cloud access settings were not changed. Exact infer: false persistence, absence of record merging and the real Holo cross-session recall loop remain unverified.

ACP-over-HTTP disconnection, standalone trust revocation, MCP restart while approval is pending and a PreToolUse ask Hook were not exercised. Cross-workspace cwd migration and Conversations remain outside this first profile. All local test daemons, MCP processes, synthetic services and temporary directories were stopped or removed, and the browser test tab was closed. No real credential, private endpoint or memory content is included in this report.

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR! Re-running the gate on the same commit — nothing about the diff moved, but CI has now landed and I corrected one number from my previous pass.

Template looks good ✓ — every required heading is present, including the three Risk & Scope bullets, the Tested-on table and the Chinese translation.

Problem — real, and I checked both halves against the pre-PR source rather than taking the description's word for them:

  • The write half: integrations/external-context-mem0 ships two entry points today, main.ts (search) and auto-recall.ts, and neither can create a record. There is no write route in the published Extension, so the gap is a plain fact about the package, not a claim.
  • The Web Shell half reproduces against baseline. On main, getPermissionContent falls back to [{ type: 'text', text: fallback || 'Tool permission' }] where fallback is perm.title, and ToolApproval then gates the body on contentText && contentText !== request.title. So a generic MCP approval whose compact-JSON title equals its body renders no body at all — the arguments the daemon already sent are invisible to the person being asked to approve. That is an observed defect in code I read, not a theoretical one.

Direction — aligned, with one thing stated precisely. #9951 is still open and carries need-discussion; this PR is not a closing reference for it (closingIssuesReferences is empty) and says so outright. What it does is land one opt-in slice — explicit write — through the existing configurable-Extension architecture instead of the built-in-provider shape the issue title proposes, plus a design doc under docs/design/ recording that choice. Additive, admin-enabled, no Core scheduling change and no new daemon route, so it does not foreclose the wider discussion in #9951. That is the right way to take a slice of an issue that is still being argued about.

Size — not applicable to the core-module gate. No changed file matches the enumerated core patterns (packages/core/src/**, packages/*/src/{auth,providers,models,config,tools,services}/**): the integration lives under integrations/, and the Web Shell edits are under packages/web-shell/client/, not src/. Only one packages/*/ tree is touched, so it is not a cross-package change either. Stage 1e matched nothing.

Correcting my previous pass, whose breakdown did not sum. The 2,526 changed lines are exactly:

Bucket Lines
Production code 494
Package unit tests 982
Daemon integration test 393
Docs (design doc + README) 429
Schema JSON 170
Test fixture 33
Example settings 25

Production sits under the 500-line maintainer-awareness threshold and well under the 1000+ large-PR advisory. Test-to-production is roughly 2.8:1, which is the shape you want for a module that writes to an external service.

Approach — the scope feels right, and two decisions are the ones I would have made:

  • It reuses instead of duplicating. validateEndpoint, validateStaticPath, resolveRepositoryRoot, requireScopeValue, readRequiredEnvironment, readConfigFile and isWithinRepository come from the existing loader; applyAuthentication and readBoundedBody from the search engine. Eight functions went from private to export and two signatures widened, but I checked the only widening that could have shifted existing behavior — validateEndpoint(instance: InstanceConfigBase) reads just endpoint.origin and endpoint.allowInsecureHttp, both already on the base interface, so V2/V3 search is untouched. applyAuthentication's new structural parameter { dialect: { auth }; credential } is satisfied by both existing runtime types.
  • write-main.ts as a third bundled entry point, wired through workspace settings rather than qwen-extension.json, is exactly how auto-recall.ts is already deployed. The default Extension stays search-only, so nobody who has not deliberately configured a writer sees any change.

Two questions, neither a blocker:

  1. This carries two user-visible features — the writer and the Web Shell argument preview. They are causally linked (without the display fix the writer's approval body is invisible), and the description says so, but the Web Shell change would land and revert independently of the integration. Was splitting it considered?
  2. The new preview predicate is not MCP-scoped, while the description scopes it to "other MCP tools without dedicated previews". I traced the actual blast radius and it is smaller than it first looks — exec permissions still take the command branch, ask-user permissions are routed to AskUserQuestion and never reach ToolApproval, and explicit content or a diff preview short-circuit before the JSON fallback. What is left is any other approval with no content block and an explicit rawInput/input/args. That is arguably an improvement, but the code and the description should agree about what it covers.

Moving on to code review. 🔍

中文说明

感谢贡献!在同一个 commit 上重跑分诊——diff 没有变化,但 CI 已经跑完,另外我修正了上一轮的一个数字。

模板 完整 ✓ —— 所有必需标题都在,包括 Risk & Scope 的三条、Tested on 表格和中文翻译。

问题 —— 真实存在,而且两半我都是对着改动前的源码核实的,没有只采信描述:

  • 写入这一半:integrations/external-context-mem0 目前只有 main.ts(搜索)和 auto-recall.ts 两个入口,都无法创建记录。已发布的 Extension 里确实没有写入路径,所以这个缺口是包的客观事实,不是一个说法。
  • Web Shell 这一半可以对着基线复现。在 main 上,getPermissionContent 会回退成 [{ type: 'text', text: fallback || 'Tool permission' }],其中 fallback 就是 perm.title;而 ToolApproval 又用 contentText && contentText !== request.title 来决定是否渲染正文。于是紧凑 JSON 标题与正文相同的通用 MCP 审批会完全不显示正文——daemon 已经送来的参数,对正要批准的人是不可见的。这是我在代码里读到的已存在缺陷,不是理论推演。

方向 —— 对齐,但有一点要说准确。#9951 目前仍是 open 且带 need-discussion 标签;本 PR 并不是它的 closing reference(closingIssuesReferences 为空),PR 自己也明确说了。它做的是落地其中一个显式启用的切片——显式写入——走的是现有可配置 Extension 架构,而不是 issue 标题提议的内置 provider 形态,并在 docs/design/ 下记录了这一选择。纯增量、由管理员启用、不改 Core 调度、不新增 daemon 路由,因此不会替 #9951 那场更大的讨论做决定。对一个仍在争论中的 issue,这是切片的正确方式。

规模 —— 核心模块规则不适用。没有改动文件命中枚举的核心路径(packages/core/src/**packages/*/src/{auth,providers,models,config,tools,services}/**):integration 位于 integrations/ 下,Web Shell 的改动位于 packages/web-shell/client/ 而非 src/。并且只触及一个 packages/*/ 目录,因此也不构成跨包改动。Stage 1e 也未命中。

修正上一轮那份加不起来的构成。2,526 行改动的准确构成是:

分类 行数
生产代码 494
包内单元测试 982
daemon 集成测试 393
文档(设计文档 + README) 429
schema JSON 170
测试 fixture 33
示例配置 25

生产代码低于 500 行的维护者关注阈值,也远低于 1000+ 的大 PR 提示线。测试与生产代码约为 2.8:1,对一个会向外部服务写入的模块来说,这是希望的形态。

方案 —— 范围合理,其中两个决定换成我也会这么做:

  • 复用而非重复。validateEndpointvalidateStaticPathresolveRepositoryRootrequireScopeValuereadRequiredEnvironmentreadConfigFileisWithinRepository 来自现有加载器;applyAuthenticationreadBoundedBody 来自搜索引擎。八个函数由私有改为 export,两个签名放宽,但我核对了唯一可能改变既有行为的那处——validateEndpoint(instance: InstanceConfigBase) 只读 endpoint.originendpoint.allowInsecureHttp,两者本来就在基接口上,所以 V2/V3 搜索没有受影响。applyAuthentication 新的结构化参数 { dialect: { auth }; credential } 也被两个既有 runtime 类型满足。
  • write-main.ts 作为第三个打包入口、通过 workspace settings 而非 qwen-extension.json 接线,与 auto-recall.ts 已有的部署方式完全一致。默认 Extension 仍只提供搜索,未显式配置 writer 的用户看不到任何变化。

两个问题,都不是阻塞项:

  1. 这个 PR 带了两个用户可见的功能——writer 和 Web Shell 参数预览。两者有因果关系(没有这个展示修复,writer 的审批正文就是不可见的),描述里也说明了,但 Web Shell 这部分本可以独立于 integration 合入与回滚。有没有考虑过拆开?
  2. 新的预览判定条件并没有限定 MCP,而描述把范围限定为"其他没有专用预览的 MCP 工具"。我追了一下实际影响面,比第一眼看上去小——exec 审批仍走命令分支,ask-user 审批被路由到 AskUserQuestion、根本不会进入 ToolApproval,显式 content 与 diff 预览也会在 JSON 回退之前短路。剩下的是其他"没有 content block 且带显式 rawInput/input/args"的审批。这可以说是改进,但代码与描述对覆盖范围的表述应当一致。

进入代码审查 🔍

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

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head d489e29. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

permission-panel-dark before/after

permission-panel-light before/after

Full-resolution recordings (.webm) are attached to the workflow run.

Qwen Code · web-shell visuals

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Code review

Re-read the whole diff against the pre-PR source on this re-run. No Critical blockers. One suggestion, one carried-over question, one maintainability ask, and a list of things I specifically tried to break and could not.

Suggestion — a received 4xx is classified unknown, not failed. In write-request-engine.ts the response branch is if (!response.ok) { await response.body?.cancel()…; return { status: 'unknown' } }. For a timeout, a dropped body, a 5xx, or a 3xx surfaced by redirect: 'manual', unknown is right — a commit genuinely cannot be ruled out. But a received 400/401/403/404/413/422 means the provider processed and rejected the request, so nothing was stored, and failed ("not submitted — check the content and administrator configuration before trying again") is the accurate one. This is not hypothetical here: the Risk section records that real Holo conformance stopped at a preflight HTTP 403 with zero creates. Both statuses set isError: true, so the failure still surfaces — the cost is that a misconfigured credential is reported to the model as "the memory may have been stored, do not retry", which discourages the legitimate retry once an administrator fixes it. Conservative is the correct default for a non-idempotent write, so I would treat this as a follow-up, not a gate.

Carried from Stage 1 — the preview predicate is wider than the described scope. getPermissionContent now returns a JSON argument dump for any permission with empty content, an explicit rawInput/input/args, and no diff preview; it never checks the tool name. I traced the routing to bound it: exec permissions still take the isExec && command branch, isAskUserPermission diverts ask-user cards to AskUserQuestion before ToolApproval ever sees them, and explicit content or a diff preview short-circuit ahead of the fallback. So the reachable set is "other approvals with no content block and explicit input" — smaller than the raw predicate suggests, and arguably an improvement, but the code and the description should agree. One cosmetic consequence worth knowing: getDescriptionText still returns request.title when there is no description, so in the exact case this fixes the card renders the compact JSON as the description line and the pretty-printed JSON as the body.

Maintainability ask — the write dialect's *Location fields are obeyed at load time, not at request time. The engine tests scope.userId !== undefined and ignores dialect.create.userIdLocation. That is correct today, and the reason is two files away: requireScopeValue is an XNOR — if ((location === 'omit') === (value === undefined)) return; — so omit with a value is a load-time ConfigurationError, and write-config.ts runs it for all three scope fields. The equivalence also depends on the write dialect's scopeLocation enum being exactly ["json", "omit"]; if query were ever added to write-dialect.schema.json, the engine would silently place it in the JSON body instead. A one-line comment at the call site naming that invariant would save the next reader the reconstruction.

What I tried to break and could not:

  • An unscoped write is not expressible. write-instance-config.schema.json requires scope with minProperties: 1, and the XNOR above forces the dialect to declare json for exactly the fields present. So at least one of user_id/agent_id/app_id always goes on the wire — there is no configuration that writes to a global corpus.
  • No auth kind is silently dropped. AuthenticationKind is authorization-token | authorization-bearer | x-api-key — all header-based — and the write dialect schema pins auth to the same three. applyAuthentication's header-only application is therefore complete for the write path; there is no query-parameter kind that could have been lost when the search engine's placeValue machinery was left behind.
  • The URL cannot be redirected or traversed. validateEndpoint rejects a userinfo, query, hash or path-bearing origin and requires HTTPS unless allowInsecureHttp === true; validateStaticPath rejects //, ?, #, \, %, control characters and ./.. segments; the dialect schema pins create.path to ^/ with a 512-char bound. basePath.replace(/\/$/u, '') before the join is what keeps /v1/ + /memories from producing a double slash that validateStaticPath would have rejected anyway.
  • Exactly one request, and the failure taxonomy matches the send point. Everything up to and including JSON.stringify(body) sits in the first try, returning failed — nothing was submitted. The single fetcher(url, init) call and its response handling sit in the second, returning unknown. There is no retry, no loop, and no second call site anywhere in the diff.
  • A refused startup never reads the secret. loadWriteRuntimeConfiguration reads credentialEnv's value last, after schema validation, endpoint and path validation, all three scope checks, and the resolveRepositoryRoot + isWithinRepository(cwd) binding. write-config.test.ts pins the ordering ("requires a credential only after the entire valid binding is loaded", "rejects an outside cwd and symlink escape without reading a credential").
  • Config and response reads are bounded. readConfigFile opens O_RDONLY | O_NONBLOCK, requires a regular file (so a FIFO or directory is rejected), caps at 64 KiB, and throws only non-disclosing ConfigurationError messages; write-main.ts prints those and a generic fallback, never a path or a stack. readBoundedBody caps at 1 MiB and cancels the stream on overflow.
  • repositoryRoot is canonicalized but never re-read — checked, and not a gap. auto-recall.ts re-reads runtime.instance.autoRecall.repositoryRoot per hook because that process serves many workspaces. The writer is spawned per workspace with a fixed cwd, so the load-time binding is the whole check; the canonical value stored back on the instance is simply unused. Worth knowing when reading, not worth changing.
  • The preview cannot spoof, inject, or blow up the card. JSON.stringify handles quotes and C0 controls; the added [\u007f-\u009f\u2028\u2029\p{Cf}] replacement covers DEL/C1, line and paragraph separators, and format characters including bidi overrides and tag characters, and it splits to code units so an astral \p{Cf} becomes a surrogate-pair escape that JSON.parse recombines correctly. The result is rendered as React text inside a <pre>, and .content clamps to three lines with max-height: 300px until hover, so a large argument dump cannot dominate the dialog. transcriptAdapter.test.ts asserts JSON.parse round-tripping, that _meta/status/toolCallId never become a preview, and that explicit content and diff previews still win; ToolApproval.test.tsx asserts no <b> element is created.
  • No dangling ARIA IDREF. commandId is the id on both the command <pre> and the content <pre>, and showsCommandBlock — which gates the aria-describedby entry — is now (isExec && command) || showsContent, exactly the condition under which one of those two elements renders. The new test asserts describedBy contains the preview's id.
  • contentIsInput is not a dead switch. Declared optional in adapters/types.ts, set only by getPermissionContent, read only by showsContent in ToolApproval.tsx; extractPendingPermission is the sole producer of a PermissionRequest in the client.
  • structuredContent matches the declared outputSchema for all four statuses. renderRememberResult spreads the result and adds message; the zod object is .strict() with exactly status/memoryId?/providerOperationId?/message, and each RememberResult variant supplies only keys in that set.

Write lifecycle

sequenceDiagram
    participant P1 as Model
    participant P2 as context_remember tool
    participant P3 as Daemon permission gate
    participant P4 as Write request engine
    participant P5 as Mem0 compatible provider
    P1->>P2: content (at most 4000 code points)
    P2->>P2: validate content, else failed
    P2->>P3: approval required (ask rule, trust false)
    P3-->>P2: approved by a human voter
    P2->>P4: remember(content, signal)
    P4->>P5: one POST to create path, infer false, fixed scope
    P5-->>P4: response, timeout, or dropped body
    P4->>P4: classify stored / accepted / failed / unknown
    P4-->>P2: result, isError on failed and unknown
    P2-->>P1: never retried automatically
Loading
Files changed (all 27 shown)
File What changed
docs/design/external-context-mem0-explicit-write.md New design doc recording the extension-profile choice this PR makes instead of the built-in provider shape #9951 proposed.
integration-tests/cli/external-context-mem0-daemon-write.test.ts The daemon-level regression: permissions and provider writes stay bound to the owning workspace, a reload touches only that writer, a rejected vote cannot write, a cancelled call's stale vote cannot write. See Testing — no PR lane currently runs it.
integrations/external-context-mem0/README.md Documents the writer: configuration, permission and result semantics, deployment check.
integrations/external-context-mem0/examples/managed-daemon-write-workspace-settings.json Worked settings wiring the writer as its own MCP server with an ask rule, includeTools and trust false. The tool name in the ask rule matches the server name the code registers.
integrations/external-context-mem0/package.json Adds src/write-main.ts to the esbuild entry points and dist/write-main.js to files.
integrations/external-context-mem0/schemas/write-dialect.schema.json Strict write dialect: create path pinned to a leading slash, scope placements limited to json or omit, response completion and id mapping, and an if/then forcing collection results when completion is records-or-event.
integrations/external-context-mem0/schemas/write-instance-config.schema.json Strict V4 instance config: additionalProperties false, scope minProperties 1, timeoutMs 100 to 30000, credentialEnv pinned to an uppercase env-var shape.
integrations/external-context-mem0/src/config.ts Exports six validators for reuse and widens validateEndpoint to InstanceConfigBase. No logic change — the bodies are context lines.
integrations/external-context-mem0/src/manifest.test.ts Asserts the packaged files list now includes the writer bundle.
integrations/external-context-mem0/src/request-engine.ts Exports applyAuthentication and readBoundedBody and widens the auth parameter structurally. No logic change.
integrations/external-context-mem0/src/schemas.ts Compiles the two new schemas, adds the two parse functions, and generalizes parseInstance to the V4 type so the basePath and allowInsecureHttp defaults still get applied.
integrations/external-context-mem0/src/types.ts Exports InstanceConfigBase and adds V4 config, the write dialect, the write runtime, and the Remember result and provider types.
integrations/external-context-mem0/src/write-config.test.ts Eight cases on load order and rejection: V4 only, canonical cwd before any credential read, symlink escape, mismatched scope placement, relative dialect paths, nonregular, oversized and malformed files.
integrations/external-context-mem0/src/write-config.ts Loads and validates the writer binding, reusing the existing validators, and reads the credential last.
integrations/external-context-mem0/src/write-main.ts Third stdio entry point; fails startup with a non-disclosing message and a nonzero exit code.
integrations/external-context-mem0/src/write-mcp.test.ts Tool registration and annotations, content validation, the failed and unknown renderings, and cancellation not initiating another write.
integrations/external-context-mem0/src/write-mcp.ts Registers context_remember with non-idempotent annotations and a description that tells the model never to retry an unknown or accepted write.
integrations/external-context-mem0/src/write-profile.ts Content bound (4000 code points, no lone surrogates, must be non-blank), the zod input and output schemas, and the four result messages.
integrations/external-context-mem0/src/write-request-engine.test.ts Astral code-point preservation, cancellation before submission versus failure after it, and request-construction errors staying local.
integrations/external-context-mem0/src/write-request-engine.ts Builds and sends the single POST and classifies the response into stored, accepted, failed or unknown.
integrations/external-context-mem0/src/write.integration.test.ts Drives the packaged bundle over stdio to HTTP: exact content sent once, and startup failure outside the configured workspace without disclosing paths.
integrations/external-context-mem0/test/fixtures/synthetic-write-v1.json Synthetic write dialect fixture.
packages/web-shell/client/adapters/transcriptAdapter.test.ts Four new cases: all three input field names, title-equal body no longer hidden, explicit content and diff previews preserved, and toolCall metadata never rendered as a preview.
packages/web-shell/client/adapters/transcriptAdapter.ts Falls back to the escaped literal arguments when a permission carries no content, factors out the explicit-input lookup, and returns the new flag alongside the blocks.
packages/web-shell/client/adapters/types.ts Adds the optional contentIsInput flag.
packages/web-shell/client/components/messages/ToolApproval.test.tsx Renders through the real adapter for the title-equal case and checks aria-describedby wiring, Escape rejecting, and markup not being interpreted.
packages/web-shell/client/components/messages/ToolApproval.tsx Shows the body when it is literal input, even when it equals the title.

Testing

This comment carries CI evidence only, quoted from the check-run API for the reviewed commit. This is an issue_comment-triggered run, so per the gate's rules I did not build, run, or execute anything from this PR — no local tmux capture, and every result below is GitHub's, not mine.

Unlike my previous pass, CI has now landed: no check failed, and the substantive ones are green. The part that matters most is which of the new tests those green checks actually executed, so I checked the wiring rather than assuming it.

What did run. integrations/external-context-mem0 is an npm workspace and defines a test:ci script, so the Test (ubuntu-latest, Node 22.x) job — which runs npm run test:ci:workspaces — executed the package suite, including all 982 new lines: write-config.test.ts, write-mcp.test.ts, write-request-engine.test.ts, the updated manifest.test.ts, and write.integration.test.ts, which builds and drives the packaged dist/write-main.js bundle over stdio to HTTP. That is the load-bearing coverage for the writer's config, validation, cancellation and exact-body-once behaviour, and it is green. The Web Shell adapter and component tests ran in the same job, and Capture web-shell visuals plus web-shell E2E Smoke are green.

What did not run. The 393-line integration-tests/cli/external-context-mem0-daemon-write.test.ts was not executed by any check on this PR. Integration Tests (CLI, No Sandbox) — the lane that collects integration-tests/cli — is merge_group-only and shows as skipped here, and ci.yml notes it never triggers until merge queue is enabled on main. The pull-request lane Integration Tests (no-AK, No Sandbox) runs an explicit allowlist of 6 files out of the 42 in integration-tests/cli/, and this PR does not extend it. To be fair to the author: that is the repo's normal arrangement, not an oversight — 36 of those 42 files sit in the same position — and the test is genuinely no-AK eligible (fake OpenAI server, local synthetic provider, skipIf on win32 and sandbox), so it could be added to that allowlist with one path if a maintainer wants these claims gated per-PR. The consequence for this review is narrow but real: the daemon-level assertions behind Test Plan steps 2–4 — cross-workspace vote rejection, reload scoping, and a cancelled call's stale vote being unable to write — are asserted by a test that has not executed anywhere yet.

Check Conclusion
Capture web-shell visuals (ubuntu-latest, Node 22.x) ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
Lint & Static (ubuntu-latest, Node 22.x) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success
Classify PR ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Two more things about the shape of that signal:

  • Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) are skipped, so the suite ran on Linux only. The author's Tested-on table marks macOS as tested and Windows and Linux as not tested — meaning the platform where CI runs is not the platform where the author ran it, and Windows is covered by neither.
  • tmux-testing and verify are both skipped on this head. There is a verification acknowledgement in this thread pointing at an in-flight orchestration run, but no verify report has been posted and the check on this commit still reads skipped, so there is nothing for me to cite. review-pr was still in progress when I fetched; it is bot orchestration, not PR CI, and the Qwen Code CI and Web-shell Visuals workflow runs for this head are both completed/success.

Not verified, and why:

  • not verified: real-service conformance. The author states plainly that Holo conformance stopped at a preflight HTTP 403 with zero creates, so exact infer: false storage, preservation of existing records, and the real cross-session recall loop are unverified against a live provider. Everything green here is against synthetic loopback services.
  • not verified: the daemon-level workspace-ownership, reload-scoping and stale-vote claims — the test that asserts them is not in any lane that ran (above).
  • not verified: Windows and macOS runtime behavior.
  • The author's E2E report in this thread (182 Mem0 tests, 67 Web Shell tests, one daemon integration test, 18 real-daemon scenarios, macOS 26.6.2) is the author's claim, not evidence I re-ran or can attest to. It is unusually candid about its own limits — it says outright that a request count is not a claim of real-service persistence — which is why I am comfortable relying on it as a description of intent while still marking the behaviour unverified.

Sandboxed verification would settle the two claims static review and this CI cannot: @qwen-code /verify — that an approved call produces exactly one POST carrying the unchanged text with infer: false and the configured scope, and that a dropped response or a transport killed after submission yields unknown with no replay, is A/B load-bearing behaviour that a green suite would not disprove if the single-send guard were removed, and it would also exercise the daemon integration test that no PR lane ran. And @qwen-code /tmux — that a generic MCP approval whose compact JSON title equals its body now renders the complete literal arguments, including whitespace and Unicode, is a TUI/Web Shell surface claim whose visual half Capture web-shell visuals covers only indirectly. The author has write access (the head branch is in-repo, not a fork), so neither lane needs sponsoring.

中文说明

代码审查

这次重跑我把整个 diff 又对着改动前的源码读了一遍。没有 Critical 阻塞项。 一条建议、一个从 Stage 1 带过来的问题、一条可维护性请求,以及一批我特意去攻破但没攻破的地方。

建议 —— 收到的 4xx 被归类为 unknown 而不是 failed write-request-engine.ts 的响应分支是 if (!response.ok) { await response.body?.cancel()…; return { status: 'unknown' } }。对超时、响应体丢失、5xx,以及 redirect: 'manual' 暴露出来的 3xx,unknown 是对的——确实无法排除已提交。但收到的 400/401/403/404/413/422 意味着 provider 处理并拒绝了请求,什么都没存,准确的分类是 failed("未提交——请先检查内容和管理员配置再重试")。这对本 PR 不是假设:Risk 一节就记录了真实 Holo conformance 停在前置 HTTP 403、create 为零。两种状态都会置 isError: true,所以失败仍然会暴露——代价是:凭证配错这种情况会被上报成"记忆可能已保存,请勿重试",从而在管理员修好之后阻碍本该发生的重试。对非幂等写入而言保守是正确的默认,所以我会把它当作后续项,而不是关卡。

从 Stage 1 带过来 —— 预览判定条件比描述的范围更宽。 getPermissionContent 现在会对任何 content 为空、带显式 rawInput/input/args、且没有 diff 预览的审批返回 JSON 参数转储,而它从不检查工具名。我追了路由来界定影响面:exec 审批仍走 isExec && command 分支,isAskUserPermission 会在 ToolApproval 看到之前就把 ask-user 卡片分流给 AskUserQuestion,显式 content 与 diff 预览也会在回退之前短路。所以可达集合是"其他没有 content block 且带显式输入的审批"——比判定条件本身看上去小,而且可以说是改进,但代码与描述应当一致。另有一个值得知道的表层后果:没有 descriptiongetDescriptionText 仍返回 request.title,所以在本次修复正针对的那个场景里,卡片会把紧凑 JSON 作为描述行渲染一次,再把格式化 JSON 作为正文渲染一次

可维护性请求 —— 写入 dialect 的 *Location 字段是在加载期生效,而不是在请求期。 引擎判断的是 scope.userId !== undefined,忽略了 dialect.create.userIdLocation。今天这是正确的,但原因在两个文件之外:requireScopeValue 是一个 XNOR——if ((location === 'omit') === (value === undefined)) return;——所以 omit 配一个值会在加载期抛 ConfigurationError,而 write-config.ts 对三个 scope 字段都调用了它。这个等价关系还依赖写入 dialect 的 scopeLocation 枚举恰好是 ["json", "omit"];如果哪天 write-dialect.schema.json 加了 query,引擎会静默地把它放进 JSON body。在调用处加一行注释点明这个不变量,能省掉下一位读者的重建工作。

我试图攻破但没攻破的地方:

  • 无 scope 的写入无法表达。 write-instance-config.schema.json 要求 scopeminProperties: 1,上面的 XNOR 又强制 dialect 对恰好那些存在的字段声明 json。所以 user_id/agent_id/app_id 至少有一个一定会上线——不存在能写入全局语料库的配置。
  • 没有 auth 类型会被静默丢弃。 AuthenticationKindauthorization-token | authorization-bearer | x-api-key——全部基于 header——写入 dialect schema 也把 auth 限定为同样三种。因此对写入路径来说,只用 header 的 applyAuthentication 是完备的;不存在某个 query 参数类型会在舍弃搜索引擎的 placeValue 机制时丢失。
  • URL 无法被重定向或穿越。 validateEndpoint 拒绝带 userinfo、query、hash 或路径的 origin,并要求 HTTPS(除非 allowInsecureHttp === true);validateStaticPath 拒绝 //?#\%、控制字符以及 ./.. 段;dialect schema 把 create.path 限定为 ^/ 且不超过 512 字符。拼接前的 basePath.replace(/\/$/u, '') 正是避免 /v1/ + /memories 产生双斜杠的那一步(尽管双斜杠本来也会被 validateStaticPath 拒掉)。
  • 恰好一次请求,且失败分类与发送点对应。 直到 JSON.stringify(body) 为止的所有代码都在第一个 try 里,返回 failed——尚未提交。唯一的 fetcher(url, init) 调用及其响应处理在第二个 try 里,返回 unknown。整个 diff 中没有重试、没有循环、没有第二个调用点。
  • 启动被拒时永不读取密钥。 loadWriteRuntimeConfiguration 最后才读 credentialEnv,此前已完成 schema 校验、endpoint 与路径校验、三项 scope 检查,以及 resolveRepositoryRoot + isWithinRepository(cwd) 绑定。write-config.test.ts 钉住了这个顺序("requires a credential only after the entire valid binding is loaded"、"rejects an outside cwd and symlink escape without reading a credential")。
  • 配置与响应读取都有界。 readConfigFileO_RDONLY | O_NONBLOCK 打开、要求常规文件(因此 FIFO 或目录会被拒)、上限 64 KiB,且只抛不泄露信息的 ConfigurationErrorwrite-main.ts 打印这些消息和一个通用兜底,从不打印路径或堆栈。readBoundedBody 上限 1 MiB,超限时取消流。
  • repositoryRoot 被规范化但从未再读——我查过,这不是缺口。 auto-recall.ts 每次 hook 都会重读 runtime.instance.autoRecall.repositoryRoot,因为那个进程服务多个 workspace。writer 是按 workspace 以固定 cwd 拉起的,所以加载期绑定就是全部检查;存回 instance 的规范值只是没被用到。读代码时值得知道,但不值得改。
  • 预览无法伪造、注入或撑爆卡片。 JSON.stringify 处理引号与 C0 控制字符;新增的 [\u007f-\u009f\u2028\u2029\p{Cf}] 替换覆盖 DEL/C1、行与段分隔符,以及包括双向覆盖和 tag 字符在内的格式字符,并且它拆分到 code unit,因此 astral 的 \p{Cf} 会变成 JSON.parse 能正确重组的代理对转义。结果作为 React 文本渲染在 <pre> 内,.content 在 hover 之前限制为三行、max-height: 300px,所以很大的参数转储不会占满对话框。transcriptAdapter.test.ts 断言 JSON.parse 往返、_meta/status/toolCallId 永不成为预览、显式 content 与 diff 预览仍然优先;ToolApproval.test.tsx 断言不会创建 <b> 元素。
  • 没有悬空的 ARIA IDREF。 commandId 同时是命令 <pre> 和正文 <pre> 的 id,而决定 aria-describedby 是否引用它的 showsCommandBlock 现在是 (isExec && command) || showsContent,恰好就是这两个元素之一会渲染的条件。新增测试断言 describedBy 包含预览的 id。
  • contentIsInput 不是死开关。adapters/types.ts 中声明为可选,只由 getPermissionContent 设置,只由 ToolApproval.tsxshowsContent 读取;extractPendingPermission 是客户端里唯一生产 PermissionRequest 的地方。
  • 四种状态的 structuredContent 都与声明的 outputSchema 匹配。 renderRememberResult 展开 result 并加上 message;zod 对象是 .strict(),字段恰好为 status/memoryId?/providerOperationId?/message,而每个 RememberResult 变体只提供该集合内的键。

测试

本条评论只携带 CI 证据,均取自被审 commit 的 check-run API。 本次由 issue_comment 触发,因此按关卡规则我没有构建、运行或执行本 PR 的任何代码——没有本地 tmux 截图,下面所有结果都是 GitHub 的,不是我的。

与上一轮不同,CI 已经跑完:没有检查失败,实质性的检查都是绿的。 最关键的是这些绿色检查究竟执行了哪些新测试,所以我去核对了接线,而不是想当然。

确实跑了的。 integrations/external-context-mem0 是一个 npm workspace 且定义了 test:ci 脚本,因此运行 npm run test:ci:workspacesTest (ubuntu-latest, Node 22.x) 任务执行了该包的测试套件,包含全部 982 行新增测试:write-config.test.tswrite-mcp.test.tswrite-request-engine.test.ts、更新后的 manifest.test.ts,以及 write.integration.test.ts——后者会构建并通过 stdio 到 HTTP 驱动打包后的 dist/write-main.js。这是 writer 的配置、校验、取消与"精确正文只发一次"行为的关键覆盖,它是绿的。Web Shell 的 adapter 与组件测试在同一任务中运行,Capture web-shell visualsweb-shell E2E Smoke 也是绿的。

没有跑的。 393 行的 integration-tests/cli/external-context-mem0-daemon-write.test.ts 没有被本 PR 的任何检查执行。收集 integration-tests/cli 的那条通道 Integration Tests (CLI, No Sandbox) 只在 merge_group 触发,在这里显示为 skipped,且 ci.yml 注明在 main 启用 merge queue 之前它根本不会触发。pull request 通道 Integration Tests (no-AK, No Sandbox) 跑的是 integration-tests/cli/ 42 个文件中显式列出的 6 个,本 PR 没有扩充这个列表。为作者说句公道话:这是仓库的常规安排,不是疏漏——那 42 个文件里有 36 个处于同样状态——而且这个测试确实符合 no-AK 条件(fake OpenAI server、本地合成 provider、win32 与 sandbox 下 skipIf),所以如果维护者希望这些主张按 PR 把关,加一条路径就能把它放进那个列表。对本次审查的影响很具体但不大:Test Plan 第 2–4 步背后的 daemon 级断言——跨 workspace 投票拒绝、reload 只影响该 writer、被取消调用的旧审批票无法写入——是由一个至今尚未在任何地方执行过的测试来保证的。

关于这个信号形态,还有两点:

  • Test (macos-latest, Node 22.x)Test (windows-latest, Node 22.x) 都是 skipped,所以套件只在 Linux 上跑过。作者的 Tested on 表格标注 macOS 已测、Windows 与 Linux 未测——也就是说 CI 运行的平台不是作者本地运行的平台,而 Windows 两边都没覆盖。
  • 这个 head 上 tmux-testingverify 都是 skipped。本 thread 里有一条验证确认评论指向一个在飞的编排 run,但验证报告尚未发布,该 commit 上的检查仍显示 skipped,所以我没有可引用的东西。取数时 review-pr 仍在进行;它是机器人编排而非 PR CI,而这个 head 的 Qwen Code CIWeb-shell Visuals 两个 workflow run 都已 completed/success。

未验证,以及原因:

  • 未验证: 真实服务的 conformance。作者明确说明 Holo conformance 停在前置 HTTP 403、create 为零,因此 infer: false 原样保存、不改写已有记录、以及真实跨会话召回闭环,都没有对着活的 provider 验证过。这里所有绿色结果都来自 loopback 合成服务。
  • 未验证: daemon 级的 workspace 归属、reload 范围与旧审批票主张——断言它们的测试不在任何已运行的通道里(见上)。
  • 未验证: Windows 与 macOS 运行时行为。
  • 作者在本 thread 中的 E2E 报告(Mem0 182 项测试、Web Shell 67 项、daemon 集成测试 1 项、18 组真实 daemon 场景、macOS 26.6.2)是作者的主张,不是我重跑过或能背书的证据。它对自身局限异常坦白——直接写明请求计数不等于真实服务持久化的主张——这也是我愿意把它当作意图描述来参考、同时仍把行为标为未验证的原因。

沙箱验证可以补上静态审查和这次 CI 补不上的两个主张:@qwen-code /verify —— 一次获批调用恰好产生一次 POST、携带未改写的正文与 infer: false 及配置的 scope,以及提交后丢弃响应或终止 transport 会得到 unknown 且不重放,这是 A/B 承重行为,即使把"只发一次"的保护去掉绿色套件也不会反驳它;它同时会执行那条没有任何 PR 通道跑过的 daemon 集成测试。以及 @qwen-code /tmux —— 紧凑 JSON 标题与正文相同的通用 MCP 审批现在能渲染完整字面参数(含空白与 Unicode),这是一个 TUI/Web Shell 表层主张,Capture web-shell visuals 只能间接覆盖它的可视部分。作者具备写权限(head 分支在仓库内,不是 fork),两条通道都不需要担保。

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

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — solid, strictly additive, and honest about its own limits; CI is green on this commit and the approval from the earlier pass still stands. Two non-blocking items and one evidence gap named below.

This is a re-run at the same commit, so let me say what actually changed rather than repeat myself. CI landed: Test, Lint & Static, Integration Tests (no-AK, No Sandbox), web-shell E2E Smoke, Capture web-shell visuals and both Desktop Shell legs are green, and my previous pass's "I cannot say the tests pass" is now retired. I also corrected a size breakdown that did not sum, and I found one thing the earlier pass got wrong by omission — it credited the daemon integration test as evidence without checking whether any lane runs it. None does. That is worth knowing and is not the author's fault; details are in Stage 2.

Stepping back to the proposal I wrote before opening the diff: asked to give daemon users an explicit memory write without touching Core, I would have added a third bundled entry point beside main.ts and auto-recall.ts, wired it through workspace settings rather than qwen-extension.json so the default Extension stays search-only, reused the config validators and the auth/bounded-body helpers instead of forking them, POSTed once with infer: false and an administrator-fixed scope, and classified the outcome so a non-idempotent write is never auto-retried. That is the shape of this PR, and I did not find a simpler path it missed. The one place I thought I had it — "surely the dialect's scope locations should be read at request time" — turned out to be an invariant enforced two files away at load time, which is the only thing in this diff that made me go back and re-read rather than forward.

What I like most is what it does not do. No Core scheduling change, no new daemon route, no default enablement, no migration. Nobody who has not deliberately configured a writer sees any difference, and the eight functions it opened up were widened in visibility only — I checked the two signatures that could have shifted behavior and neither did. The security posture is consistent end to end, and the parts I would normally expect an opt-in first profile to get wrong are the parts it got right: an unscoped write is not expressible, a refused startup never reads the credential, the failure taxonomy is anchored to whether the request actually left the process, and the text a human is asked to approve is escaped before it is displayed.

Two things I would change, neither of which should hold this up:

  • A received 4xx is classified unknown rather than failed. Timeouts, dropped bodies and 5xx genuinely are ambiguous, but a 403 the provider actually returned is a rejection — and the PR's own Risk section says the real Holo attempt died on exactly that. The failure still surfaces as an error, so this costs diagnosability rather than safety, and the conservative direction is the right default for a non-idempotent write. But "may have been stored, do not retry" is the wrong thing to tell someone whose credential was simply misconfigured.
  • The Web Shell argument preview is broader than the description claims. I traced the routing and the reachable set is smaller than the raw predicate suggests — exec and ask-user cards never get there, and explicit content and diff previews still win — so this is about matching the code to the stated intent, plus a cosmetic wrinkle where the compact JSON title now renders as the description line and the pretty JSON as the body.

What I could not confirm, stated plainly. The real-service half of this feature is unverified — the author says so, with a 403 and zero creates, and everything green is against synthetic loopback services. The daemon-level ownership, reload-scoping and stale-vote assertions have not executed anywhere, because the test that carries them is in the merge-queue/release lane and this PR does not add it to the per-PR no-AK allowlist. Windows is untested locally and in CI. That is an acceptable place to land an opt-in, admin-configured first profile that calls itself a first profile; it is not an acceptable place to land if this is going to be pointed at a production memory service on the strength of this review.

Six months from now I would thank whoever wrote this rather than curse them. The writer is a separate, small, strictly-configured module that reuses the validators already in the package, and the tests pin the ordering and the rejection cases rather than just the happy path. My only maintainability ask is the one-line comment at the scope-placement call site, so the next reader does not have to reconstruct a load-time invariant from a different file the way I did.

Verdict: approve — and the approval already stands. My earlier pass deferred it behind an approval-deferral marker because the main CI run had not landed; the finalize job then posted the approval pinned to d489e295f5697e0dce5252f4049736afe6b6cc4d at 15:38 UTC. The head has not moved since, so that vote is still against the code I re-reviewed, and this run posts no second approval — one per commit. No marker this time either: there is nothing left to defer to. The approval guardrail does not apply (in-repo branch, feat not refactor), and Stage 0 raised no escalation.

What would move this to 5/5 is not more review, it is the sandboxed lane: @qwen-code /verify for the exactly-one-POST and no-replay claims — which would also finally execute that daemon integration test — and @qwen-code /tmux for the Web Shell preview surface. A verify acknowledgement is already in this thread and its report will land separately; read it against the two unverified claims above rather than as a general blessing.

中文说明

信心:4/5 —— 扎实、纯增量、对自身局限诚实;这个 commit 上 CI 已全绿,且上一轮的批准仍然有效。下面列出两个非阻塞项和一个证据缺口。

这是在同一个 commit 上的重跑,所以我只说真正变化的部分,不重复自己。CI 跑完了:TestLint & StaticIntegration Tests (no-AK, No Sandbox)web-shell E2E SmokeCapture web-shell visuals 以及两条 Desktop Shell 都是绿的,上一轮那句"我不能说测试通过"现在可以撤回了。我还修正了一份加不起来的规模构成,并且发现上一轮因遗漏而说错了一处——它把 daemon 集成测试当作证据来记,却没有核对是否有任何通道真的运行它。答案是没有。这一点值得知道,而且不是作者的过错;细节在 Stage 2。

退回到我打开 diff 之前写下的方案:要在不触碰 Core 的前提下给 daemon 用户一个显式记忆写入,我会在 main.tsauto-recall.ts 旁边加第三个打包入口,通过 workspace settings 而不是 qwen-extension.json 接线,让默认 Extension 保持只读搜索,复用配置校验器与 auth/有界响应体读取的辅助函数而不是另抄一套,以 infer: false 和管理员固定 scope 发出一次 POST,并对结果分类,使非幂等写入永不被自动重试。这正是本 PR 的形态,我没有找到它漏掉的更简路径。唯一一处我以为自己抓到了什么的地方——"dialect 的 scope 位置当然应该在请求期读取"——结果是两个文件之外、加载期强制的不变量;这也是整个 diff 里唯一让我回头重读而不是继续往下走的东西。

我最欣赏的是它没有做的事。没有改动 Core 调度,没有新增 daemon 路由,没有默认启用,没有迁移。没有刻意配置 writer 的用户看不到任何差别,而它开放出来的八个函数只放宽了可见性——我核对了两个可能改变行为的签名,都没有变。安全姿态前后一致,而且通常一个"显式启用的首版 profile"最容易做错的那几处,恰好是它做对的:无 scope 的写入无法表达,启动被拒时永不读取凭证,失败分类锚定在"请求是否真的离开了进程",以及要人来批准的文本在展示前先转义。

有两处我会改,但我都不认为应该因此卡住:

  • 收到的 4xx 被归类为 unknown 而非 failed。超时、响应体丢失、5xx 确实是模糊的,但 provider 真实返回的 403 是一次拒绝——而 PR 自己的 Risk 一节就写了真实 Holo 尝试正是死在这里。失败仍然会以错误形式暴露,所以代价是可诊断性而非安全性,且对非幂等写入而言保守方向是正确的默认。但对于只是凭证配错的人,"可能已保存,请勿重试"是错误的提示。
  • Web Shell 参数预览比描述所述更宽。我追了路由,可达集合比判定条件本身看上去小——exec 与 ask-user 卡片根本到不了那里,显式 content 与 diff 预览仍然优先——所以这属于让代码与所述意图对齐的问题,外加一个表层小瑕疵:紧凑 JSON 标题现在会作为描述行渲染一次,格式化 JSON 又作为正文渲染一次。

我无法确认的部分,直说。这个功能的真实服务那一半未经验证——作者说明了,403、create 为零,而所有绿色结果都来自 loopback 合成服务。daemon 级的归属、reload 范围与旧审批票断言至今没有在任何地方执行过,因为承载它们的测试位于 merge-queue/release 通道,而本 PR 没有把它加进按 PR 运行的 no-AK 列表。Windows 在本地和 CI 都未测。对于一个显式启用、由管理员配置、并且自称是首版 profile 的改动,这是可以接受的落地状态;但如果要凭这次审查就把它指向生产环境的记忆服务,那就不可接受。

六个月后我会感谢写这段代码的人,而不是咒骂他。writer 是一个独立、体积小、配置严格的模块,复用了包里已有的校验器,而且测试钉住的是顺序与各种拒绝场景,不只是顺利路径。我唯一的可维护性请求,就是那个 scope 位置调用处的一行注释,好让下一位读者不必像我这样从另一个文件里把一个加载期不变量重建出来。

结论:批准——而且批准已经生效。 上一轮因为主 CI 尚未跑完,把批准挂在了一个批准延后标记后面;随后 finalize 任务提交了绑定到 d489e295f5697e0dce5252f4049736afe6b6cc4d 的批准,时间是 UTC 15:38。此后 head 没有移动,所以那一票仍然对应我这次重审的代码,本次运行不提交第二次批准——每个 commit 只批一次。这次也没有标记:已经没有什么需要延后了。批准护栏不适用(仓库内分支,是 feat 而非 refactor),Stage 0 也没有触发升级。

能把这个分数提到 5/5 的不是更多审查,而是沙箱通道:@qwen-code /verify 用来验证"恰好一次 POST"与"不重放"这两个主张——它同时会终于执行那个 daemon 集成测试——以及 @qwen-code /tmux 用来看 Web Shell 预览表层。本 thread 里已经有一条 verify 确认评论,其报告会单独发布;请把它对照上面两个未验证的主张来读,而不是当作一次笼统的背书。

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

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

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship — CI landed green after the review. ✅

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ⚠️ inconclusive — completed without a usable structured 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: 399 passed · 0 failed · 399 total

Flakiness gate: ✅ 7 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定:⚠️ 无法判定 · 已完成但无可用的结构化判定

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:399 通过 · 0 失败 · 399 总计

抖动门:✅ 7 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR #11311 deep verification — feat(external-context): Add daemon memory writes

Verdict: merge-ready — 399 scripted assertions executed, 399 passed / 0 failed.
Verified head OID d489e295f5697e0dce5252f4049736afe6b6cc4d (git rev-parse HEAD^2), base a1cbe75cf1dc94cfe9544488304af9e7f76ab28a (HEAD^1, equal to the snapshot's baseRefOid), merge commit 80e3d1c77855db5577357fc012ee96027edf4791.
Three non-blocking items below: one correction to the description and two test-coverage Suggestions. No blocking finding.

中文摘要

结论:merge-ready 399 条脚本断言全部通过,0 失败。验证 head 为 d489e295,base 为 a1cbe75c

A/B 结论。 三条主张都用真实产物做了 A/B,不是读代码推断:

  • 中心主张(写入器):base 树按 base 自己的 build 脚本构建后 dist/ 只有 main.jsauto-recall.js,没有 write-main.js,直接加载会 ERR_MODULE_NOT_FOUND;head 的真实打包产物 dist/write-main.js 作为真实 MCP stdio 子进程启动,经真实 loopback HTTP provider 收到恰好一次 POST,正文与提交文本逐字节相同(含首尾空白、换行、制表符、NBSP、中文、emoji),infer:false,scope 与 dialect 一致。见 01-write-ab-head-has-writer-base-does-not.png
  • 共享面回归:PR 改动了 config.ts / request-engine.ts / schemas.ts / types.ts,这些是既有检索路径依赖的模块。360 个 dialect 组合下,base 与 head 的 wire 请求(method、URL、全部 header、body 原文)逐字节相同,0 处差异;配置接受/拒绝判定也完全一致。见 02-search-regression-ab-360-wire-cells-identical.png
  • Web Shell 预览:8/8 个通用 MCP 审批形态从「正文被隐藏」翻转为「正文显示」;显式 content 预览、diff 预览、纯元数据 toolCall、rawInput 优先级等回归对照在两臂完全一致。见 04-webshell-preview-ab-8-flips-hidden-to-shown.png

Findings(均非阻塞)。 详见下文 Findings 与 Corrections 两节,此处只列标题:新的参数预览落进既有的 3 行 CSS 截断、仅 :hover 展开(对描述的更正);write-main.tsprocess.exitCode = 1 无测试固定(覆盖缺口);rawInput > input > args 优先级在 PR 自带测试中无固定(覆盖缺口,且会影响 exec 审批框显示的命令)。

变异矩阵。 21 个守卫逐一回退,19 个被杀,未变异对照全绿(mem0 写入相关 94 项 + Web Shell 67 项);2 个存活者都用更细的行为探针判定为覆盖缺口而非死代码或冗余防御。见 03-mutation-matrix-19-of-21-killed.png

未覆盖范围。 真实 Mem0/Holo 服务一致性(PR 自述 preflight 403)、infer:false 在真实服务上的保存语义、跨会话召回闭环(Reviewer Test Plan 第 5 步)、浏览器实际渲染(截断结论来自样式表与 adapter 输出,非 Chrome 测量)、Windows/macOS、仓库级 lint/format。完整清单见 Not covered

Scope selection

The PR bundles two independent changes. I named one central claim and two secondary ones, and everything else is listed under Not covered.

# Claim Source How it was tested
C1 (central) An opt-in, administrator-bound writer submits the exact supplied text once, as POST with infer: false and exactly the dialect-declared scope, and classifies the outcome as stored / accepted / failed / unknown without automatic retry. PR title, "What this PR does" H1 (end-to-end wire oracle), H7 (annotations + replay), mutation rows M1–M14
C2 Web Shell shows complete literal arguments for generic MCP permissions lacking a dedicated content/diff preview, including when the argument text equals the title. "What this PR does", Reviewer Test Plan step 1 H2 (A/B flip matrix), H6 (rendering limits), mutation rows M15–M19, M-COMB
C3 Existing V2 search and V3 Auto Recall configurations retain their meanings; the default Extension remains search-only. "What this PR does", "Risk & Scope" H3 (360-cell wire-identity A/B), manifest inspection

No injection attempt was found in the PR title, body, commit message, or code comments. Author claims were treated as hypotheses throughout; two of them ("182 Mem0 package tests", "67 related Web Shell tests") were checked and are exactly correct.

C1 — central claim: A/B table

Both arms are real builds. The base arm is a git worktree at HEAD^1 built with the base tree's own build script (which bundles only src/main.ts and src/auto-recall.ts). The head arm is the artifact CI already produced at HEAD — dist/write-main.js as shipped, not a rebuild.

Cell Environment Observable oracle Base (a1cbe75c) Head (d489e295)
Writer entry exists built dist/ of each tree existsSync + direct node <entry> absent; ERR_MODULE_NOT_FOUND, exit ≠ 0 present, starts, advertises context_remember
One exact write real MCP stdio child + real loopback node:http provider recorded method / path / headers / raw body n/a (no writer) 1 request: POST /api/memories, authorization: Token …, body messages:[{role:"user",content:<byte-identical>}], infer:false, user_id:"repository-memory", no agent_id/app_id
Literal-text fidelity same body.messages[0].content === content (JS identity, not equality-by-normalisation) n/a passes for ' literal\n中文 😀 "quote"\t\u00a0 END ' and for 2000 astral chars
No automatic retry same provider.requests.length n/a 1 after each of 17 distinct unknown outcomes, and 1 after cancel and after timeout
Outcome classification same structuredContent.status n/a 17/17 unknown cells, 4/4 accepted/stored cells, 15/15 sibling-sweep cells correct
Admission guards same status + request count n/a 4000 cp → stored/1 request; 4001 cp → failed/0 requests; blank / control-only / Cf-only / lone surrogate → failed/0 requests; extra arguments (user_id, infer, url) → rejected by the strict schema, 0 requests
Fail-closed startup same connect rejects + requests.length + stderr n/a 6/6 rejected with 0 provider requests and no path/credential/scope-value disclosure

Witness: 01-write-ab-head-has-writer-base-does-not.png (cells 1–3 and the auth matrix as they printed). Raw log: logs/h1-write-wire.log (214 assertions).

The base arm is a genuine absence, not a stub: dist/ was produced by running the base commit's own package.json build script, so the control differs from head by exactly the PR's change to that script.

C1 supporting measurements

  • Auth matrix (H1): all three dialect kinds produce the right header on the wire — Token cred-abc, Bearer cred-abc, and x-api-key: cred-abc with no authorization header.
  • Scope biconditional (H1): requireScopeValue enforces location === 'omit' iff the instance value is absent. I probed this because it is what makes the engine's if (scope.userId !== undefined) correct — the engine never consults dialect.create.*Location itself. Both illegal combinations (omit + value present, json + value absent) are rejected at startup with 0 provider requests, and the error does not leak the rejected value. Mutation M11 (loosening the biconditional) is killed by both the new write-config.test.ts and the pre-existing schemas.test.ts.
  • Concurrency (H1): 3 in-flight calls → 3 requests, 3 distinct memory ids, each carrying its own exact text and infer:false. No cross-talk.
  • Cancel after submission (H1): the provider had already received the write; cancelling produced no second request and never reported stored.
  • Replay suppression (H7 + Core gate): the shipped artifact advertises readOnlyHint:false, idempotentHint:false, destructiveHint:false, openWorldHint:true. Core's canSafelyReplay() (packages/core/src/tools/mcp-tool.ts:507) returns false for those annotations under every trust configuration I evaluated, including trust:true in a trusted folder. The empirical half is Core's own suite: packages/core/src/tools/mcp-tool.test.ts -t 'auto-reconnect on connection error'27 passed, 80 skipped, exit 0 (logs/gate-core-replay.log). So the README's claim that the annotations prevent transparent replay is verified, not assumed.
  • Default Extension stays search-only (C3): qwen-extension.json is untouched and starts only dist/main.js with includeTools: ["context_search"]. A repo-wide grep finds write-main referenced only by the build script, the files list, the example settings, and the tests — never by the manifest.

C2 — Web Shell preview: A/B table

Both arms are the real compiled adapter, bundled by esbuild from each tree. The module has only type-only imports, so each bundle is self-contained; source hashes differ (40d4095f… head vs aa40293b… base), confirming the control is genuine.

Cell group Oracle Base Head
Flip matrix (8 shapes: rawInput/input/args, title == JSON, title ≠ JSON, nested, 4000 cp, whitespace-heavy) the component's own display predicate contentText && (contentIsInput || contentText !== title) body hidden in 8/8 body shown in 8/8, contentIsInput: true
Losslessness JSON.parse(displayedText) vs the supplied arguments n/a exact round-trip, pretty-printed at indent 2
Escaping regex census over the rendered text n/a no raw \u007f-\u009f, no \u2028/\u2029, no \p{Cf}; RLO → \u202e; astral tag U+E0001 → \udb40\udc01; lone surrogate → \ud800; CJK/emoji/NBSP not over-escaped; round-trip still lossless
Regression controls (explicit text content, diff preview, empty content array, metadata-only toolCall, undefined toolCall) base vs head deep equality identical identical, no argument leakage in either arm
getPermissionRawInput refactor (10 precedence shapes) base vs head deep equality identical identical — matters because this value feeds getCommandFromRawInput() (the command shown in an exec approval) and getDescriptionText()

Witness: 04-webshell-preview-ab-8-flips-hidden-to-shown.png. Raw log: logs/h2-preview-ab.log (84 assertions).

M-COMB (layered guards). contentIsInput in the adapter and showsContent in the component are two hunks defending one hazard from different layers, so I added the combination row the single-hunk rows cannot see. Reverted together: 5 tests red across both files. Each alone is also killed (M15 → 4 red, M18 → 1 red), so both are independently load-bearing rather than redundant.

C3 — shared-surface regression: A/B table

The PR edits four modules the existing retrieval path depends on. A clean A/B here needs a real confound removed first:

The integration pins ajv 8.20.0 in its own node_modules, while the repository root has ajv 6.15.0. A base worktree has no node_modules of its own and would silently resolve root ajv 6 — a different major, which would have made the control meaningless. I symlinked the base tree's integration node_modules to the head's and then asserted the realpath from the esbuild metafile: both arms resolve the identical integrations/external-context-mem0/node_modules/ajv and …/json-schema-traverse. The symlink was removed before the worktree was torn down, and the head tree's ajv was re-verified present at 8.20.0 afterwards.

Cell group Oracle Result
Control integrity esbuild metafile input list head arm = 8 source inputs, all HEAD-TREE, 2 write schemas; base arm = 6 source inputs, all BASE-TREE, 0 write schemas; identical ajv realpath
Search wire identity 360 dialect combinations (method × 5 scope-location triples × 3 limit fields × 4 threshold/rerank × 3 auth kinds), each driven through a real loopback provider on both arms 0 differing cells out of 360 on method, URL, every header, and raw body; 0 differing normalized results; 360/360 produced real items
Not a both-broken tie 14 spot assertions on what the wire must contain POST /api/search, Bearer cred-zzz, query/user_id in body, filters.agent_id, no app_id when omit, top_k: 5, threshold/rerank forwarded; GET variant carries everything in searchParams with an empty body and x-api-key
Schema decisions parseInstanceConfig on 12 fixtures, parseDialect on 6 identical accept/reject and identical error messages on both arms; positive control confirms at least one rejection, and a write dialect is rejected by the search parser on both arms
Config loading loadRuntimeConfiguration against real temp files and env identical runtime object for a valid config; identical rejection for missing env, relative path, unresolved ${TOK} placeholder, and blank credential
Containment isWithinRepository on 5 candidates identical (true, true, false, false, false)

Witness: 02-search-regression-ab-360-wire-cells-identical.png. Raw log: logs/h3-search-ab.log (45 assertions).

Residual delta accounted for: the head search bundle is 3.7 KB larger than base (291,379 vs 287,516 bytes). That is exactly the two new write schemas, which schemas.ts imports and the shim re-exports — the metafile names both files as head-only inputs. No unexplained residue.

Corrections to the description

The literal-argument preview is complete in the DOM but is visually clamped to 3 lines at rest, and expands only on mouse hover. The description says Web Shell "now displays complete literal arguments", and Reviewer Test Plan step 1 asks the reviewer to "Confirm that the full literal arguments are readable". The <pre> the change routes arguments into uses styles.content, whose rule (packages/web-shell/client/components/messages/ToolApproval.module.css:130) declares display: -webkit-box; -webkit-line-clamp: 3, and the only selector that unclamps it is .content:hover — there is no :focus-within and no @media (hover: none) fallback anywhere in the module. This is a correction to the description, not a request to change the code, and it is explicitly not a defect this PR introduced:

  • the clamp is pre-existing house style, byte-identical to .codeBlock, which renders rm -rf … for exec approvals today;
  • the PR changed zero stylesheets (0 of 27 changed files are .css);
  • the full text is still present in the DOM, in the element's title attribute, and in the alertdialog's aria-describedby, so a screen-reader user hears all of it;
  • the author's own evidence line ("the final marker remained visible after scrolling") is consistent with having observed the hover-expanded state.

What is worth the reviewer's attention is the interaction with the payload this feature exists to approve, because the adapter pretty-prints at indent 2 (measured in H6): an empty {} renders as 1 line and fits; a single short context_remember fact renders as 3 lines and fits; any argument object with two or more keys renders as ≥ 4 lines and is therefore clamped at rest, as is any single-key payload whose value wraps — the PR's own 3,046-code-point example and the writer's 4,000-code-point cap both render as 3 unwrapped source lines carrying 3,065 and 4,019 code points. A touch or keyboard-only reviewer sees the first 3 rendered lines of the text they are being asked to approve. I did not run a browser, so this is measured from the shipped stylesheet plus the real adapter output and reported as "3 rendered lines", not as a visible-character count. Witness: 05-argument-preview-3-line-clamp-hover-only.png; log logs/h6-clamp.log (26 assertions).

Findings

S1 — Suggestion: write-main.ts exit code is load-bearing but unpinned

process.exitCode = 1 is the only signal a supervisor has to distinguish "writer failed to start on a configuration error" from "writer started and exited cleanly". Nothing in the PR's tests observes it.

Reproduce:

cd /__w/qwen-code/qwen-code && D=$(cat /tmp/verifydir.txt)   # or: node tmp/pr11311-verify-*/h5-survivors.mjs "$PWD" "$PWD/tmp/pr11311-verify-*"
QWEN_EXTERNAL_CONTEXT_MEM0_WRITE_CONFIG=relative/not-absolute.json \
  node integrations/external-context-mem0/dist/write-main.js; echo "exit=$?"

Measured: head prints Mem0 extension write configuration path must be absolute. and exits 1. With that single statement changed to process.exitCode = 0, stderr is byte-identical and the process exits 0 — yet write.integration.test.ts stays green at 4/4, because it asserts only that client.connect() rejects, which happens on any child exit. Classification: coverage gap, not dead code — the behaviour is real and observable, nothing asserts it. The fixture that would go red is an assertion on the spawned child's exitCode in the existing "fails startup outside the configured workspace" case, which already has the transport in hand. Evidence: logs/h5-survivors.log, logs/h4-mutations.log row M14.

S2 — Suggestion: rawInput > input > args precedence is unpinned by the shipped suite

The PR refactored getPermissionRawInput to share a new getExplicitPermissionInput helper, which now also decides what the preview shows. The precedence is load-bearing well beyond the new feature: the same value feeds getCommandFromRawInput(), i.e. the command text rendered in an exec approval dialog, and getDescriptionText().

Reproduce:

cd /__w/qwen-code/qwen-code && D=$(cat /tmp/verifydir.txt)
node "$D/h5-survivors.mjs" "$PWD" "$PWD/$D"     # reverses the precedence, rebuilds, re-runs both oracles

Measured: with the three lookups reversed to args ?? input ?? rawInput, the PR's own transcriptAdapter.test.ts stays green at 13/13 — its it.each(['rawInput','input','args']) fixture sets exactly one of the three fields per case, so nothing ever has to choose. The same mutant turns my H2 harness 6 assertions red, specifically on rawInput parity: all three present -> rawInput wins and input before args. Classification: coverage gap; the behaviour at head is correct (H2 asserts it on the unmutated tree, and asserts base/head parity across all 10 precedence shapes). The fixture that would go red is one toolCall carrying rawInput, input and args simultaneously, asserting rawInput wins — worth adding, since the current test's name claims to cover all three fields while its fixture cannot distinguish them. Evidence: logs/h5-survivors.log, logs/h4-mutations.log row M19.

O1 — Observation (fail-safe, no action needed): certain non-writes are reported as unknown

Any non-2xx maps to unknown, including 400/401/403, where the write provably did not land. The user is therefore told "The memory may have been stored. Do not retry automatically" for a request that was certainly rejected. This is the safe direction — nothing ever claims a write that did not happen, and unknown is isError: true either way — and it is consistent with the README's own table ("including after timeout, cancellation, disconnection, non-2xx status or an invalid reply"). I verified 400, 401, 302, 500 and a destroyed socket all yield unknown with exactly 1 provider request and no leaked memoryId. Recording it because it is a deliberate tradeoff a reviewer may want to confirm was intended, not because it is wrong.

Mutation matrix

Unmutated control: green — mem0 write suites 94 passed / 0 failed (4 files), Web Shell suites 61 passed / 0 failed (2 files). Without that, no kill below would mean anything. Every mutation is interface-preserving (behaviour changes, exports and types do not), applied to a byte-exact backup and restored unconditionally; git status --porcelain is empty afterwards.

Witness: 03-mutation-matrix-19-of-21-killed.png. Full log with expected/received evidence per row: logs/h4-mutations.log.

Row Guard reverted Suite that should catch it Result Intended assertion it failed
M1 infer: falsetrue write-request-engine, write.integration KILLED 3 red - "infer": false / + "infer": true in the deep-equal on the request body
M2 send content.trim() write-request-engine, write.integration KILLED 3 red - "content": " exact … " / + "content": "exact …"
M3 4000 cp cap → 400000 write-request-engine, write-mcp KILLED 2 red expected { status: 'unknown' } to match { status: 'failed' }
M4 drop \p{Cs} lone-surrogate clause write-request-engine, write-mcp KILLED 3 red same intended assertion
M5 drop blank/control-only clause write-request-engine, write-mcp KILLED 6 red same intended assertion
M6 non-2xx → failed instead of unknown write-request-engine KILLED 8 red - "status": "unknown" / + "status": "failed"
M7 length !== 1length < 1 (accept ambiguous multi-record) write-request-engine KILLED conflicting-response case
M8 identifier grammar → any non-empty string write-request-engine KILLED invalid-response case
M9 drop the completion === 'records-or-event' gate on accepted write-request-engine KILLED keeps invalid/conflicting response 19 unknown without retry
M10 drop record-level status/event checks write-request-engine KILLED keeps invalid/conflicting response 11 unknown without retry
M11 loosen the scope biconditional write-config, schemas, auto-recall-config KILLED promise resolved … instead of rejecting — caught by a pre-existing search-side test as well as the new one
M12 drop writer cwd containment write-config, write.integration KILLED 1 red expected [Function] to throw 'writer is outside its repository' but got 'credential was read'
M13 drop the absolute-config-path check write-config KILLED 1 red expected … 'path must be absolute' but got 'Mem0 extension instance configuration…'
M14 process.exitCode = 10 write.integration SURVIVED 4/4 green → adjudicated in S1 (coverage gap, proven behaviourally)
M15 drop contentIsInput: true transcriptAdapter KILLED 4 red expected undefined to be true; - "contentIsInput": true
M16 escape regex → never matches transcriptAdapter KILLED 3 red expected '{ "content": " 😀…' not to match /[\u0085\p{Cf}]/u
M16b escape class narrowed to DEL only transcriptAdapter KILLED 3 red same — so C1, bidi and tag characters are each independently pinned
M17 drop !hasPermissionDiffPreview(...) transcriptAdapter KILLED 1 red expected true to be undefined (diff preview must not leak arguments)
M18 showsContentcontentText !== title ToolApproval.test.tsx KILLED 1 red renders generic parameter content even when it equals the title: expected undefined to be '{}'
M19 reverse rawInput > input > args transcriptAdapter SURVIVED 13/13 green → adjudicated in S2 (coverage gap; H2 kills it)
M-COMB M15 and M18 reverted together both Web Shell files KILLED 5 red the layered-guard row: proves the set is load-bearing, and that neither hunk alone is redundant defence

19/21 killed, 2 survivors, both adjudicated as coverage gaps — no dead code and no redundant defence among them. No mutation hit an anchor error, so every row really applied. Per the repository's own review rule, the two survivors are completeness reporting, not merge conditions: neither is load-bearing for correctness at head, and both behaviours were independently confirmed correct by H1/H2/H5.

Gates

All green, run from a known-clean state at head.

Gate Command Result
Mem0 package suite cd integrations/external-context-mem0 && ../../node_modules/.bin/vitest run --config vitest.config.ts 182 passed / 0 failed, 13 files — matches the description's "182 Mem0 package tests" exactly
Web Shell affected files cd packages/web-shell && ../../node_modules/.bin/vitest run client/adapters/transcriptAdapter.test.ts client/components/messages/ToolApproval.test.tsx client/components/messages/ToolApproval.test.ts 67 passed / 0 failed, 3 files — matches the description's "67 related Web Shell tests" exactly
Daemon E2E (the PR's own) cd integration-tests && QWEN_SANDBOX=false ../node_modules/.bin/vitest run cli/external-context-mem0-daemon-write.test.ts 1 passed in 8.8 s against a real spawned daemon, real workspace settings, real MCP child and a fake model server
Core replay gate cd packages/core && ../../node_modules/.bin/vitest run src/tools/mcp-tool.test.ts -t 'auto-reconnect on connection error' 27 passed, 80 skipped
Typecheck (mem0) cd integrations/external-context-mem0 && tsc --noEmit exit 0, no output
Typecheck (web-shell) cd packages/web-shell && tsc -p tsconfig.json --noEmit exit 0, no output

The gates are proven live rather than assumed: the mutation matrix above turns the same suites red 19 separate ways, including one row (M11) that reddens a pre-existing test file the PR never touched. An exit-0 gate that cannot fail was not accepted as evidence anywhere in this report.

The daemon E2E is the strongest single piece of coverage for Reviewer Test Plan step 3, and it passed here. Its assertions include: both workspaces see their own toolCall.rawInput; a cross-workspace vote is refused (vote(runA, pB) === false) with the provider request count staying at 0; reject_once leaves it at 0; approval produces exactly the expected request body; a second vote on an already-resolved permission returns false; after restarting only workspace B's MCP server, only B's next write uses the new binding; and a stale vote on a cancelled call returns false with the count frozen at 4.

Not covered

  • Real provider conformance. No Mem0/Holo/production-compatible service was contacted. The PR itself reports preflight HTTP 403 with zero creates, and lists infer: false storage semantics, preservation of existing records, and the real cross-session recall loop as unverified. Everything here proves the writer's handling of synthetic provider bytes, not a real service's behaviour — this reproduces the wire shape, not a real corpus.
  • Reviewer Test Plan step 5 (reader configured against the same corpus; save via one client, recall via another client in a new session; another workspace's scope must not return it) — not attempted, since it requires a real provider corpus.
  • Reviewer Test Plan steps 1, 2 and 4 were covered indirectly rather than driven as written: step 1's "no request while pending or after rejection" and step 2's "exactly one POST" come from H1 and the daemon E2E; step 1's "full literal arguments are readable" comes from H2/H6 without a browser (see Corrections); step 4's "terminate the MCP transport and confirm Core skips unsafe replay" comes from H7's measured annotations plus Core's own pinned suite, not from a live transport kill during a daemon write. No step turned out to be unperformable.
  • No browser rendering anywhere. The 3-line-clamp finding is derived from the shipped stylesheet and the real adapter output. I did not measure visible characters, scroll behaviour, tooltip rendering, or the actual line-wrap width in Chrome.
  • Windows and macOS. Linux container only (node:22-bookworm, Node v22.23.2). The description marks both as not tested by the author too.
  • Repo-wide lint, format, and the full root test suite. Not run; only the affected workspaces were typechecked and tested, per the targeted-gate contract. The PR's own CI covers the rest.
  • Per-commit attribution. Not applicable and verified as such: the snapshot's commits array has exactly 1 entry (d489e295), matching git rev-list HEAD^1..HEAD^2. I did not rely on rev-list --count alone, since the repository is shallow (git rev-parse --is-shallow-repositorytrue) and that call returns a plausible small number at a graft boundary instead of erroring.
  • The 265-line design doc (docs/design/external-context-mem0-explicit-write.md) was not audited line by line against the implementation; only the README claims I tested are vouched for.
  • The daemon E2E was run once. I did not repeat it to characterise flakiness, and it contains no timing-triggered assertion whose margin I measured.
  • The base arm's dependency tree. The PR changes no package.json dependency and no lockfile entry (only the mem0 scripts.build and files list), so reusing the installed tree is a clean control; I still had to equalise ajv resolution explicitly, as described under C3.

Methodology

Environment: the CI verify job's own container — node:22-bookworm, Node v22.23.2, no zstd, $QWEN_VERIFY_CONTEXT at /__w/_temp/verify-context/pr.json, working tree at refs/pull/11311/merge (depth 2), with npm ci and npm run build already completed at HEAD. No GitHub token and no network calls were made; the local tree plus the metadata snapshot were the whole world, and nothing was posted anywhere.

Seven harnesses (h1h7, all .mjs in this directory so a maintainer can rerun them verbatim) drove the real code rather than a model of it. H1 and H7 spawn the shipped bundle dist/write-main.js as a real child process, talk to it over a real MCP stdio transport using the real @modelcontextprotocol/sdk Client, and point it at a real loopback node:http provider that records every byte it receives; nothing on the path is stubbed, and the provider enforces the upstream semantics the claim depends on (stalling, destroying the socket mid-response, emitting a partial body, exceeding the 1 MiB cap, returning ambiguous or non-ADD records). H2, H3 and H6 run esbuild-bundled copies of the real source from each tree — head from the repository root, control from a scratch git worktree at HEAD^1 — and compare their outputs cell by cell; H3 additionally drives 360 dialect combinations through real sockets on both arms and diffs method, URL, every header and the raw body. H4 and H5 mutate the head tree in place against byte-exact backups, run the affected vitest files, and restore unconditionally, with the restore verified by an empty git status --porcelain.

Every number in assertions.json (399 pass / 0 fail) comes from a scripted comparison that executed and could have failed; A/B control cells are encoded as assertions that the base arm fails, so an expected base-arm red counts as a pass and fail contains only unexpected outcomes. Gate counts (182 + 67 + 1 + 27 vitest tests, 2 clean tsc runs) are reported separately and are not folded into that 399. Raw per-cell stdout/stderr lives in logs/, the esbuild metafiles that prove dependency resolution in bundles/meta-{head,base}.json, and the rendered witnesses in evidence/. The five PNGs were produced with node scripts/verify-capture.mjs; four render the tail of a saved live-run log and one (03) renders a grep-condensed view of the mutation log, so each image shows the exact bytes a real execution printed. The scratch worktree was removed with git worktree remove --force after its dependency symlink was deleted first, and the head tree's node_modules was re-verified intact afterwards.

Flakiness gate log

integration test, out of gate scope: integration-tests/cli/external-context-mem0-daemon-write.test.ts
rounds=5 files=7 skipped=1
file integrations/external-context-mem0/src/manifest.test.ts: (cd integrations/external-context-mem0) npx --no-install vitest run ./src/manifest.test.ts
file integrations/external-context-mem0/src/write-config.test.ts: (cd integrations/external-context-mem0) npx --no-install vitest run ./src/write-config.test.ts
file integrations/external-context-mem0/src/write-mcp.test.ts: (cd integrations/external-context-mem0) npx --no-install vitest run ./src/write-mcp.test.ts
file integrations/external-context-mem0/src/write-request-engine.test.ts: (cd integrations/external-context-mem0) npx --no-install vitest run ./src/write-request-engine.test.ts
file integrations/external-context-mem0/src/write.integration.test.ts: (cd integrations/external-context-mem0) npx --no-install vitest run ./src/write.integration.test.ts
file packages/web-shell/client/adapters/transcriptAdapter.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/adapters/transcriptAdapter.test.ts
file packages/web-shell/client/components/messages/ToolApproval.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/messages/ToolApproval.test.tsx


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  integrations/external-context-mem0/src/manifest.test.ts: PPPPP
  integrations/external-context-mem0/src/write-config.test.ts: PPPPP
  integrations/external-context-mem0/src/write-mcp.test.ts: PPPPP
  integrations/external-context-mem0/src/write-request-engine.test.ts: PPPPP
  integrations/external-context-mem0/src/write.integration.test.ts: PPPPP
  packages/web-shell/client/adapters/transcriptAdapter.test.ts: PPPPP
  packages/web-shell/client/components/messages/ToolApproval.test.tsx: PPPPP

verdict: pass
summary: 7 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · integrations/external-context-mem0/src/manifest.test.ts: P (exit 0)
round 1 · integrations/external-context-mem0/src/write-config.test.ts: P (exit 0)
round 1 · integrations/external-context-mem0/src/write-mcp.test.ts: P (exit 0)
round 1 · integrations/external-context-mem0/src/write-request-engine.test.ts: P (exit 0)
round 1 · integrations/external-context-mem0/src/write.integration.test.ts: P (exit 0)
round 1 · packages/web-shell/client/adapters/transcriptAdapter.test.ts: P (exit 0)
round 1 · packages/web-shell/client/components/messages/ToolApproval.test.tsx: P (exit 0)
round 2 · integrations/external-context-mem0/src/manifest.test.ts: P (exit 0)
round 2 · integrations/external-context-mem0/src/write-config.test.ts: P (exit 0)
round 2 · integrations/external-context-mem0/src/write-mcp.test.ts: P (exit 0)
round 2 · integrations/external-context-mem0/src/write-request-engine.test.ts: P (exit 0)
round 2 · integrations/external-context-mem0/src/write.integration.test.ts: P (exit 0)
round 2 · packages/web-shell/client/adapters/transcriptAdapter.test.ts: P (exit 0)
round 2 · packages/web-shell/client/components/messages/ToolApproval.test.tsx: P (exit 0)
round 3 · integrations/external-context-mem0/src/manifest.test.ts: P (exit 0)
round 3 · integrations/external-context-mem0/src/write-config.test.ts: P (exit 0)
round 3 · integrations/external-context-mem0/src/write-mcp.test.ts: P (exit 0)
round 3 · integrations/external-context-mem0/src/write-request-engine.test.ts: P (exit 0)
round 3 · integrations/external-context-mem0/src/write.integration.test.ts: P (exit 0)
round 3 · packages/web-shell/client/adapters/transcriptAdapter.test.ts: P (exit 0)
round 3 · packages/web-shell/client/components/messages/ToolApproval.test.tsx: P (exit 0)
round 4 · integrations/external-context-mem0/src/manifest.test.ts: P (exit 0)
round 4 · integrations/external-context-mem0/src/write-config.test.ts: P (exit 0)
round 4 · integrations/external-context-mem0/src/write-mcp.test.ts: P (exit 0)
round 4 · integrations/external-context-mem0/src/write-request-engine.test.ts: P (exit 0)
round 4 · integrations/external-context-mem0/src/write.integration.test.ts: P (exit 0)
round 4 · packages/web-shell/client/adapters/transcriptAdapter.test.ts: P (exit 0)
round 4 · packages/web-shell/client/components/messages/ToolApproval.test.tsx: P (exit 0)
round 5 · integrations/external-context-mem0/src/manifest.test.ts: P (exit 0)
round 5 · integrations/external-context-mem0/src/write-config.test.ts: P (exit 0)
round 5 · integrations/external-context-mem0/src/write-mcp.test.ts: P (exit 0)
round 5 · integrations/external-context-mem0/src/write-request-engine.test.ts: P (exit 0)
round 5 · integrations/external-context-mem0/src/write.integration.test.ts: P (exit 0)
round 5 · packages/web-shell/client/adapters/transcriptAdapter.test.ts: P (exit 0)
round 5 · packages/web-shell/client/components/messages/ToolApproval.test.tsx: P (exit 0)

Evidence images

01-write-ab-head-has-writer-base-does-not

02-search-regression-ab-360-wire-cells-identical

03-mutation-matrix-19-of-21-killed

04-webshell-preview-ab-8-flips-hidden-to-shown

05-argument-preview-3-line-clamp-hover-only

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@chiga0 chiga0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #11311 Review — feat(external-context): Add daemon memory writes

Tier: Standard(管理员可选功能,无本地持久化格式变更,无 wire protocol 变更)


审查范围

文件 关注维度
write-mcp.ts MCP 工具注册、annotation、信号中止
write-request-engine.ts HTTP POST 构造、凭证处理、redirect 策略、AbortSignal 超时链
write-config.ts env var 读取、绝对路径校验、isWithinRepository 安全检查
write-profile.ts isValidMemoryContent 校验逻辑、writeOutputSchemarenderRememberResult 输出
write-main.ts 启动入口、错误处理(不回显凭证)
types.ts(integration) WriteInstanceConfigV4WriteDialectV1RememberResult 类型
schemas.ts(integration) AJV 写入配置 schema 校验
request-engine.ts applyAuthentication:凭证位置(header,不在 URL)
transcriptAdapter.ts getPermissionContent 重构、rawInput 参数展示逻辑
ToolApproval.tsx contentIsInput flag、showsContent 逻辑
types.ts(web-shell) contentIsInput?: boolean 接口扩展

结果:无阻塞问题

凭证安全(Class 4):CLEAN

applyAuthentication 仅向请求 headers 写入凭证(Authorizationx-api-key),不出现在 URL 参数、日志或错误消息中。write-main.ts 的错误处理仅输出 ConfigurationError 的通用消息,原始凭证不被回显。

写入合约对称性(Class 1):CLEAN

writeOutputSchema(Zod)与 renderRememberResult 展开 RememberResult 的结果完全吻合:stored 状态携带 memoryIdaccepted 携带 providerOperationIdfailed/unknown 无额外字段,均含 messagewriteInputSchema 使用 .strict() 拒绝未知参数,满足设计文档要求。

ToolApproval 兼容性(Class 2):CLEAN

contentIsInput 为可选字段,未设置时(undefined,falsy)完全保留旧的 contentText !== request.title 分支逻辑。getPermissionContent 改为返回 Pick<PermissionRequest, 'content' | 'contentIsInput'> 再展开,等效于原先的 content: 赋值,contentIsInput 仅在 rawInput 路径下设为 true

HTML 注入防护(Class 4):CLEAN

<pre> 内容使用 React JSX 字符串子节点渲染,不含 dangerouslySetInnerHTML,HTML 标签被正常转义。测试 keeps the complete literal parameter body available without interpreting markup 验证了 <b> 不会被解析为 HTML 元素。

redirect: 'manual' + 非 ok 响应处理:CLEAN

写入端手动处理 redirect;3xx 响应的 response.ok === false,直接返回 { status: 'unknown' },不跟随重定向,避免凭证泄漏。

isWithinRepository 安全边界:CLEAN

write-config.ts 在启动时校验 process.cwd() 是否位于配置的 repositoryRoot 内,防止跨 workspace 误用。


非阻塞观察

  • isValidMemoryContent 双重调用:在 write-mcp.tswrite-request-engine.ts 中各调用一次。属于防御性纵深设计,不影响正确性。

  • 非 BMP \p{Cf} 字符的转义展示transcriptAdapter.ts 中对 U+1D173–U+1D17A 等非 BMP 格式字符,character.split('').map(c => '\\u' + c.charCodeAt(0).toString(16)) 会展示为两个 \uXXXX 代理对序列而非单一码点。这仅影响审批弹窗的显示(极其罕见的音符控制字符),实际写入内容不受影响,round-trip 无损。


覆盖率声明

已覆盖 11 个核心文件的静态分析(凭证流、合约一致性、ToolApproval 兼容性)。未进行本地执行(工作区不可用)——类型检查和单测依赖 CI(CI bot 已报绿)。

Reviewed with AI assistance.

@doudouOUC
doudouOUC added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit f1ed3bc Sep 7, 2026
83 of 84 checks passed
@doudouOUC
doudouOUC deleted the codex/mem0-daemon-write branch September 7, 2026 16:35
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.23.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants