feat(external-context): Load administrator-owned Mem0 dialects - #10634
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
E2E test reportTested commit: Environment: macOS, Node.js 24.12.0, npm 10.9.8. Reproducible synthetic bundle test
Read-only live compatibility smoke test
中文说明E2E 测试报告测试提交: 环境:macOS、Node.js 24.12.0、npm 10.9.8。 可重复的合成 bundle 测试
只读真实兼容性烟测
|
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: this is a planned follow-up, not a speculative fix. PR #10149 intentionally shipped the extension with an empty preset registry, so it fails closed for every configuration and cannot connect any service until this step lands. The linked design (#10113) already anticipated the administrator-owned dialect path. The author's E2E report in this thread describes a real Hologres-compatible service they want to connect — a concrete need, not a theoretical one. Direction: aligned. The merged design explicitly reserved the administrator-owned custom-preset path ("must use an absolute path, pass the same closed schema and semantic validation, contain no credentials"), and this PR implements exactly that with the Size: not a core-module change — everything lives in Approach: the scope is right and the diff is minimal for the stated goal — the preset registry is swapped for an absolute Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:这是计划内的后续步骤,不是投机性修复。#10149 有意发布了空 preset registry,扩展对所有配置都 fail closed,在本步骤落地前无法接入任何服务。关联设计(#10113)已预留了管理员自有 dialect 路径。作者在本帖的 E2E 报告描述了一个真实的 Hologres 兼容服务接入需求——是具体需求,不是理论问题。 方向:对齐。已合并的设计明确预留了管理员自有 custom-preset 路径("必须使用绝对路径、通过同样的封闭 schema 与语义校验、不包含凭证"),本 PR 正是按此实现,且 规模:非核心模块变更——全部改动位于 方案:范围合理,对所述目标而言 diff 已是最小——用绝对路径 风险:无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRead the full diff against the merged skeleton. No blocking issues found — the change is exactly the swap it claims to be, and the load-bearing invariants survived it:
Two non-blocking notes:
Files changed (16 of 16 shown)
TestingCI evidence from the PR's own runs on the reviewed commit, fetched via API — nothing was re-run locally: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。
Sandboxed verification would settle the remaining gap: 中文说明代码审查对照已合并的 skeleton 完整阅读了 diff,未发现阻塞问题——改动与声明完全一致,关键不变量都保住了:
两点非阻塞备注:
测试来自该提交自身 CI 的证据,通过 API 获取——未在本地重新运行:
沙盒验证可以补上剩余缺口: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal, well-tested step that completes the extension's stated rollout; approval waits only on CI landing green. This PR does one thing and does it properly: it replaces the intentionally-empty preset registry with an administrator-owned dialect file, which is exactly the step the merged design reserved for making the extension usable without putting provider data in the Qwen repo. The approach matches what I would have proposed independently, and I did not find a simpler path it missed — the bounded reader, validation order, closed grammar, and redacted fail-closed behavior are all preserved rather than rebuilt, and the rewritten tests actually add coverage (URL/env-var dialect paths, per-file size bounds, credential ordering) instead of just being churned to compile. The two reservations, both non-blocking: the design rewrite retires the planned Qwen-maintained preset rollout entirely, which is a real product decision — it is recorded in the doc and shrinks Qwen's surface, and the author is a repo maintainer driving their own merged design series, so I flagged it for visibility in Stage 1 rather than holding the PR on it; and the end-to-end runtime claims (stdio startup, restart-only reload, live Hologres smoke test) rest on the author's report since CI here can only exercise Linux unit lanes. The Every change in the diff is necessary for the stated goal; nothing unrelated rode along. If I'm maintaining this in six months, the config loader is still one small file with fixed error categories — I'd thank the author. The Linux unit suite was still running at review time, so approval is deferred until CI lands green on 中文说明置信度:4/5 —— 干净、最小化、测试充分的步骤,完成了该扩展既定的发布路线;仅待 CI 全绿后批准。 这个 PR 只做一件事,而且做得恰当:用管理员自有的 dialect 文件替换有意留空的 preset registry——这正是已合并设计预留的、让扩展在不向 Qwen 仓库引入厂商数据的前提下可用的那一步。方案与我独立设想的做法一致,也没有找到更简单的替代路径——有界读取器、校验顺序、封闭语法、脱敏 fail-closed 行为全部被保留而非重建,重写后的测试还新增了覆盖面(URL/环境变量 dialect 路径、逐文件大小边界、凭证顺序),而不只是为了编译通过的机械改动。 两点保留意见,均非阻塞:设计文档重写整体放弃了原计划由 Qwen 维护的 preset 发布路线,这是一个真实的产品决策——它已记录在文档中、缩小了 Qwen 的暴露面,且作者是推动自己已合并设计系列的仓库维护者,因此我在 Stage 1 中提示了可见性,而不以此卡住 PR;端到端运行时声明(stdio 启动、仅重启加载、Hologres 实测烟测)依赖作者报告,因为这里的 CI 只能覆盖 Linux 单元通道。审查评论中的 diff 中每一项改动都是目标所必需,没有无关内容夹带。若六个月后由我维护,配置加载器仍是一个带固定错误分类的小文件——我会感谢作者。 审查时 Linux 单元测试仍在运行,因此批准推迟到该提交的 CI 全绿之后——finalize 任务会在所有检查绿色完成后发布绑定该提交的批准;若有红色结果,会在本线程标记。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qqqys
left a comment
There was a problem hiding this comment.
Reviewing the full diff at 39aaf6f against the merged skeleton: no blocking issues found.
- The empty preset registry is cleanly replaced by the administrator-owned
dialectPathflow: instance file → absolute-path check → bounded 64 KiB read of the dialect file → schema + semantic validation → credential read last. Fail-closed ordering is preserved. - Error categories stay fixed and leak-free (instance vs dialect, unavailable vs invalid);
readConfigFilebounds apply to both files. dialect.idcorrectly becomes an audit label with no lookup semantics, and the new tests pin the v1/v3 rejection, oversize bounds for both files, relative/${...}/URL dialect paths, and scope consistency.- No consumers of the removed
presets.jsremain outside the files this PR updates.
CI is green on this head (72 completed check-runs, all success/skipped; only the automatic review lane still running), matching the triage approval.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| throw new ConfigurationError( | ||
| 'Mem0 extension configuration is unavailable.', | ||
| 'Mem0 extension instance configuration is unavailable.', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R1-1: Credential failures are mislabeled as instance-configuration failures. The shared helper readRequiredEnvironment serves two call sites — the instance-config path read (config.ts:28, CONFIG_ENV) and the credential read (config.ts:51, instance.credentialEnv) — but this diff specialized its message to the "instance configuration" category, so a missing/blank/unresolved credential env var is now misreported as an unavailable instance configuration. Before this diff the message was generic ("configuration is unavailable") and covered both call sites, and the design doc's rewritten fixed-category taxonomy names instance, dialect, endpoint, path, and scope categories but no credential category.
An administrator deploys valid instance and dialect files, but the credential variable named by credentialEnv (e.g. MEMORY_API_KEY) is unset, blank, or left as the unresolved ${MEMORY_API_KEY} placeholder. Startup fails closed correctly, but stderr prints Mem0 extension instance configuration is unavailable. — the operator re-checks the instance file's existence and permissions, which are fine, instead of the missing credential. Because the design deliberately makes these fixed redacted categories the only diagnostic signal, the misdirection costs a full debugging round at deployment time.
Witness:
Probe (scratch tree; valid instance + dialect files; only the credential var unset):
unmodified PR: {"name":"ConfigurationError","message":"Mem0 extension instance configuration is unavailable."}
with fix: {"name":"ConfigurationError","message":"Mem0 extension credential is unavailable."} <- flips
Suggested fix — give readRequiredEnvironment a message/kind parameter mirroring readConfigFile(path, kind):
function readRequiredEnvironment(
env: NodeJS.ProcessEnv,
name: string,
kind: 'instance configuration' | 'credential',
): string {
const value = env[name];
const trimmed = value?.trim();
if (!value || !trimmed || trimmed === '${' + name + '}') {
throw new ConfigurationError(`Mem0 extension ${kind} is unavailable.`);
}
return value;
}Keep 'Mem0 extension instance configuration is unavailable.' (kind 'instance configuration') for the CONFIG_ENV call site and pass 'credential' at the credential call site; also add the credential category to the design doc's failure-category list. The new message must stay a fixed redacted category and must not embed the credentialEnv name or any path — docs/design/external-context-mem0-extension.md ("Startup and failure behavior") requires that messages never include the real path, endpoint, query, credential, or upstream response, and main.ts writes ConfigurationError.message verbatim to stderr. Please tighten the existing test 'rejects blank, unresolved, and missing credentials after validation' in schemas.test.ts to assert the credential-specific message — its loose .rejects.toThrow('configuration is unavailable') passes under both wordings today — and confirm the mutation: revert the credential call site to the instance-configuration message and the tightened test must go red.
中文说明
[Suggestion] R1-1:凭证缺失被误标为实例配置不可用。共享辅助函数 readRequiredEnvironment 有两个调用点——实例配置路径读取(config.ts:28,CONFIG_ENV)和凭证读取(config.ts:51,instance.credentialEnv)——但本 diff 把它的错误消息特化成了 "instance configuration" 类别,导致凭证变量缺失/为空/未解析时被误报为实例配置不可用。改动前消息是通用的("configuration is unavailable"),两个调用点都适用;设计文档重写后的固定错误类别列表列出了 instance、dialect、endpoint、path、scope 类别,却没有凭证类别。
当管理员部署了合法的实例文件和 dialect 文件,但 credentialEnv 指定的凭证变量(如 MEMORY_API_KEY)未设置、为空或仍是未解析的 ${MEMORY_API_KEY} 占位符时:启动会正确 fail closed,但 stderr 打印 Mem0 extension instance configuration is unavailable.——运维人员会去复查实例文件的存在性和权限(都没有问题),而不是缺失的凭证。由于设计上刻意让这些固定脱敏类别成为唯一的诊断信号,这种误导会在部署时浪费一整轮排查。
证据(探针):在临时树中用合法实例 + dialect 文件、仅取消设置凭证变量运行——未修复的 PR 输出 {"name":"ConfigurationError","message":"Mem0 extension instance configuration is unavailable."};应用修复后输出 {"name":"ConfigurationError","message":"Mem0 extension credential is unavailable."}——探针结果翻转。
建议修复:给 readRequiredEnvironment 增加 message/kind 参数(参照 readConfigFile(path, kind),见上方代码块),CONFIG_ENV 调用点保留 'Mem0 extension instance configuration is unavailable.'(kind 为 'instance configuration'),凭证调用点传入 'credential';同时在设计文档的错误类别列表中补充凭证类别。新消息必须是固定的脱敏类别,不得内嵌 credentialEnv 名称或任何路径——docs/design/external-context-mem0-extension.md("Startup and failure behavior")要求消息永远不包含真实路径、endpoint、query、凭证或上游响应,且 main.ts 会把 ConfigurationError.message 原样写到 stderr。请收紧 schemas.test.ts 中现有的 'rejects blank, unresolved, and missing credentials after validation' 测试:目前的宽松断言 .rejects.toThrow('configuration is unavailable') 两种措辞都能通过;改为断言凭证专用消息,并验证突变:当凭证调用点恢复为 instance-configuration 消息时该测试必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| export function createMem0McpServer(runtime: { | ||
| instance: InstanceConfigV1; | ||
| instance: InstanceConfigV2; | ||
| search: SearchProvider; | ||
| }): McpServer { |
There was a problem hiding this comment.
[Suggestion] R1-2: Runtime context_search failures have no observable category. The handler's catch (mcp.ts:58-60) collapses every provider failure into External context search failed. and writes nothing to stderr, while startup errors in main.ts do get categorized stderr lines. This item was explicitly deferred "to PR2" on the linked skeleton PR (10149) — author reply, 2026-08-27: "Deferring F1 to PR2. … should be designed with the live-provider observability work rather than widening this skeleton." This PR is the rollout's PR2 step per the Rollout section rewritten in this same diff, but ships without implementing or re-deferring the item — and the rewritten rollout deletes the live-provider preset steps the deferral pointed at.
This PR removes the empty-registry gate, so administrators now bind real services and runtime search failures become reachable for the first time. When context_search fails — expired credential (401), DNS failure, timeout, oversized or malformed response — the administrator sees only the generic result with an empty stderr and has no channel to identify the cause, diagnosing by blind trial and error.
Witness:
Probe (real MCP server over InMemoryTransport, rejecting SearchProvider):
unmodified PR: result [{"type":"text","text":"External context search failed."}], isError: true, stderr calls: []
with fix: stderr calls ["Mem0 provider request failed.\n"], tool result byte-identical <- flips
Suggested fix — write a fixed, redacted failure category to stderr in the catch before returning the redacted result:
} catch {
process.stderr.write('Provider request failed.\n');
return errorResult('External context search failed.');
}Alternatively, explicitly re-defer the item in this PR's description with a named follow-up. The stderr category must stay a fixed label that never includes the real path, endpoint, query, credential, or upstream response (docs/design/external-context-mem0-extension.md, "Startup and failure behavior"). Please add a test in mcp.test.ts that spies on process.stderr.write, rejects the SearchProvider, calls context_search, and asserts the category line was written while the tool result stays exactly External context search failed. — removing the stderr write must turn it red.
中文说明
[Suggestion] R1-2:运行时 context_search 失败没有可观察的错误类别。处理函数的 catch(mcp.ts:58-60)把所有 provider 失败都折叠为 External context search failed.,且不向 stderr 写任何内容,而 main.ts 中的启动错误却有分类的 stderr 输出。该事项在关联的骨架 PR(10149)上被明确推迟到 "PR2"——作者 2026-08-27 回复:"Deferring F1 to PR2. … should be designed with the live-provider observability work rather than widening this skeleton."。按本 diff 重写的 Rollout 章节,本 PR 正是分阶段交付的 PR2 步骤,但既未实现也未重新推迟该事项——而且重写后的 rollout 删除了该推迟所指向的 live-provider preset 步骤。
本 PR 移除了空 registry 门槛,管理员现在可以绑定真实服务,运行时检索失败首次变得可达。当 context_search 失败时——凭证过期(401)、DNS 失败、超时、响应过大或格式非法——管理员只能看到通用结果和空的 stderr,没有任何渠道区分原因,只能盲目试错排查。
证据(探针):通过 InMemoryTransport 驱动真实 MCP server、令 SearchProvider 拒绝——未修复的 PR:结果 [{"type":"text","text":"External context search failed."}],isError: true,stderr 调用:[];应用修复后:stderr 调用 ["Mem0 provider request failed.\n"],工具结果逐字节一致——探针结果翻转。
建议修复:在 catch 中返回脱敏结果之前,把一个固定的脱敏失败类别写入 stderr(见上方代码块)。或者,在本 PR 描述中明确重新推迟该事项并指定后续跟进。stderr 类别必须是固定标签,永远不包含真实路径、endpoint、query、凭证或上游响应(docs/design/external-context-mem0-extension.md,"Startup and failure behavior")。请在 mcp.test.ts 中补充测试:spy process.stderr.write、令 SearchProvider 拒绝、调用 context_search,断言写入了类别行且工具结果保持为 External context search failed.——移除 stderr 写入后该测试必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
What this PR does
This PR changes the retrieval-only
external-context-mem0Extension to load an administrator-ownedDialectV1JSON file referenced by an absolutedialectPathinInstanceConfigV2. It removes the unused built-in preset registry while preserving the existing closed request grammar, bounded response normalization, External Context MCP Profile, andcontext_searchtool surface.The instance and dialect files are each limited to 64 KiB and read once at Extension startup. Endpoint, path, scope, and dialect semantics are validated before the credential named by
credentialEnvis read. Errors remain fixed and redacted, and the package continues to contain no provider or customer dialect data.Why it's needed
PR #10149 intentionally shipped an empty preset registry, so administrators could not connect a compatible service without a new Qwen release. Administrator-owned dialect files make the existing bounded runtime usable while keeping Qwen out of the provider-preset business and keeping credentials out of Qwen settings and JSON configuration files.
Reviewer Test Plan
How to verify
InstanceConfigV2file outside the workspace that references an absoluteDialectV1file, setQWEN_EXTERNAL_CONTEXT_MEM0_CONFIGplus the named credential environment variable, and start the bundled Extension. It should initialize successfully, expose onlycontext_search, and send the exact bounded request described by the dialect with the administrator-fixed scope.presetconfiguration, a relative or URL dialect path, missing or malformed files, a file over 64 KiB, an unsupported dialect field, inconsistent scope placement, or a missing credential. Startup should fail closed with a fixed redacted category that does not reveal paths, endpoints, credentials, queries, or upstream responses.Locally verified with 49 package tests, package and root typecheck/lint/build, a real bundled stdio MCP flow against a loopback synthetic HTTP service, restart-only reload assertions, redacted startup-failure assertions, formatting checks, and
npm pack --dry-run.Evidence (Before & After)
N/A — this changes non-UI Extension configuration and runtime behavior.
Tested on
Environment (optional)
macOS with Node.js 24.12.0 and npm 10.9.8, using the bundled stdio MCP server and a local loopback synthetic HTTP service.
Risk & Scope
DialectV1grammar and treats the files and process environment as an administrative trust boundary.schemaVersion: 1configurations usingpresetnow fail closed. Administrators must migrate toschemaVersion: 2and provide an absolutedialectPath; credentials remain in the environment variable named bycredentialEnv.Linked Issues
Related: #10113, #10149
中文说明
本 PR 做了什么
本 PR 将只读检索的
external-context-mem0Extension 调整为加载管理员自有的DialectV1JSON 文件,该文件由InstanceConfigV2中的绝对路径dialectPath引用。同时删除未实际使用的内置 preset registry,并保持现有封闭请求语法、有界响应归一化、External Context MCP Profile 和context_search工具接口不变。实例文件和 dialect 文件分别限制为 64 KiB,并且只在 Extension 启动时读取一次。系统先校验 endpoint、路径、scope 和 dialect 语义,之后才读取
credentialEnv指定的凭证。错误仍使用固定脱敏分类,发布包继续不包含任何厂商或客户 dialect 数据。为什么需要
PR #10149 有意发布了空 preset registry,因此管理员无法在不等待新的 Qwen 版本时接入兼容服务。管理员自有 dialect 文件让现有有界运行时真正可用,同时避免 Qwen 维护厂商 preset,并确保凭证不进入 Qwen settings 或 JSON 配置文件。
Reviewer 测试计划
如何验证
InstanceConfigV2文件并通过绝对路径引用DialectV1文件,设置QWEN_EXTERNAL_CONTEXT_MEM0_CONFIG和指定的凭证环境变量,然后启动打包后的 Extension。它应成功初始化、只暴露context_search,并按照 dialect 发送精确的有界请求,scope 固定由管理员配置。preset配置、相对路径或 URL dialect 路径、缺失或损坏文件、超过 64 KiB 的文件、不支持的 dialect 字段、不一致的 scope 位置或缺失凭证。启动应 fail closed,并只返回固定脱敏分类,不泄露路径、endpoint、凭证、query 或上游响应。本地已验证 49 个包级测试、包级和根仓库 typecheck/lint/build、基于本地 loopback 合成 HTTP 服务的真实 bundle stdio MCP 流程、仅重启加载新配置的断言、启动失败脱敏断言、格式检查以及
npm pack --dry-run。证据(变更前后)
N/A — 本变更不涉及 UI,只调整 Extension 配置和运行时行为。
测试平台
环境(可选)
macOS,Node.js 24.12.0,npm 10.9.8;使用打包后的 stdio MCP server 和本地 loopback 合成 HTTP 服务。
风险与范围
DialectV1语法,并将这些文件和进程环境视为管理员信任边界。preset的schemaVersion: 1配置现在会 fail closed。管理员必须迁移到schemaVersion: 2并提供绝对dialectPath;凭证继续仅存在于credentialEnv指定的环境变量中。关联事项
相关:#10113、#10149