feat(external-context): Add configurable Mem0 extension skeleton - #10149
Conversation
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@doudouOUC Stopping at the gate before code review — this is a direction escalation, not a rejection of the code. The open question here is not about the implementation; it is the same one that stopped the design PR #10113 at Stage 1, and it is still unanswered.
Template: complete ✓ — all required sections, bilingual, honest about what is and isn't in scope.
Problem: real, not theoretical — #9952 shows live demand for Mem0-compatible variants, and this was already established by the #10113 gate. Not re-litigating it here.
Direction — escalating to @yiliang114: this PR is PR1 of the rollout designed in #10113, and #10113 is still open with its direction question unresolved: which route is canonical for Mem0-compatible providers — the direct polardb-mem0 integration in #9952, or this preset-based extension boundary? Three facts make that question sharper now, not weaker:
- No maintainer has decided. There has been no reply on the #10113 thread since its escalation, and the design document in this very diff states that superseding #9952 "requires an explicit maintainer decision".
- The direct route is actively advancing: #9952 was E2E-verified against a live PolarDB instance on Aug 25, and a design follow-up posted today (Aug 26) rules out the reduced
baseUrlvariant, committing further to the direct path. - This PR is stacked on #10113's unmerged design commit, so it cannot merge before PR0 lands anyway — reviewing ~2,400 lines before the route is chosen is exactly the spend the gate exists to avoid.
Also on the record: the PR touches release infrastructure (.github/workflows/release.yml generalizes the release-versioning glob to integrations/*/package.json; that file is owned by @pomelo-nwu @wenshao) — an independent reason the release-facing wiring needs maintainer eyes before merge.
Size / risk, for context: the core-module gate does not apply — nothing under packages/ is modified; all new code lives in the self-contained integrations/external-context-mem0/ tree (~850 production lines, ~740 test, ~330 schema/fixtures, the rest docs and build wiring). No high-risk paths matched. The escalation is purely directional.
What happens next: once @yiliang114 decides which route is canonical (and #10113 lands, if it is this one), re-run @qwen-code /triage here for the full Stage 2/3 review. If the direct route wins, parts of this package would be re-scoped — which is exactly why that decision should come first.
中文说明
@doudouOUC 审查在代码评审之前的门禁处停下——这是方向升级，不是对代码的否定。这里待回答的不是实现问题，而是让设计 PR #10113 停在 Stage 1 的同一个问题，而它至今没有答案。
模板：完整 ✓——必需章节齐全，中英文对照，对范围内外的陈述诚实。
问题：真实存在，不是理论假设——#9952 证明了 Mem0 兼容变体的现实需求，这一点在 #10113 的门禁中已经确认，此处不再重复论证。
方向——升级给 @yiliang114：本 PR 是 #10113 所设计分阶段方案中的 PR1，而 #10113 仍然开放，方向问题未决：Mem0 兼容 provider 的规范路线是哪一条——#9952 的直接 polardb-mem0 集成，还是本方案基于 preset 的 extension 边界？三个事实让这个问题现在更尖锐，而不是更弱化：
- 没有维护者做出过决定。升级之后 #10113 线程没有任何回应，而本 PR diff 中的设计文档自己也写明：取代 #9952 "需要维护者明确决策"。
- 直接路线正在积极推进：#9952 于 8 月 25 日在真实 PolarDB 实例上完成了 E2E 验证，今天（8 月 26 日）的设计后续说明又否定了只暴露
baseUrl的简化方案，进一步坚定了直接路线。 - 本 PR 堆叠在 #10113 未合并的设计提交之上，在 PR0 落地前本来就无法合并——在路线选定之前审查约 2400 行代码，正是门禁要避免的消耗。
另外记录在案：本 PR 触及发布基础设施（.github/workflows/release.yml 将发布版本化 glob 泛化为 integrations/*/package.json；该文件属 @pomelo-nwu @wenshao 所有）——这是发布相关改动在合并前需要维护者过目的又一独立理由。
规模 / 风险（供参考）：核心模块门禁不适用——packages/ 下没有任何改动；新增代码全部位于自包含的 integrations/external-context-mem0/ 目录（约 850 行生产逻辑、约 740 行测试、约 330 行 schema/fixture，其余为文档和构建接线）。未命中高风险路径。升级纯粹出于方向问题。
后续：等 @yiliang114 决定哪条路线为规范（若选择本路线，还需 #10113 先落地）之后，在此重新运行 @qwen-code /triage，届时进行完整的 Stage 2/3 审查。如果直接路线胜出，本 package 的部分内容需要重新划定范围——这正是该决定应当先行的原因。
— Qwen Code · qwen3.8-max
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/release.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/release.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
Author-side direction is now recorded on #10113: proceed with the Extension route. This PR remains the scoped PR1 implementation with no live presets. I will keep it unchanged and stacked until the maintainer confirms the canonical direction and PR0 lands; then triage can be re-run here. 作者侧方向已记录在 #10113：采用 Extension 路线。本 PR 继续保持为限定范围的 PR1 实现，不加入任何 live preset。在维护者确认 canonical 方向并且 PR0 合并前，将保持当前堆叠状态；之后再在此重新运行 triage。 |
|
Addressed all eight review suggestions in 0b95572.
Verification: 38/38 extension tests passed; extension typecheck, lint, build, and npm packaging dry run passed; repository-wide build, typecheck, and lint passed. Two clean diff-audit passes found no additional actionable issue. No live provider preset, write operation, Core provider branch, or other PR1 scope expansion was added. |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 120 passed · 0 failed · 120 total Flakiness gate: ✅ 5 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：120 通过 · 0 失败 · 120 总计 抖动门：✅ 5 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 10149 Deep Verification —
|
| cell | build | config | oracle | result |
|---|---|---|---|---|
| shipped | dist/main.js at HEAD |
fully valid instance + credential | exit code + stderr | exit 1, Mem0 extension preset is unknown. |
| shipped | same | missing env / relative path / missing file / malformed / >64 KiB | exit code + stderr | exit 1, cause named (5 more cells) |
| shipped | same | exactly 64 KiB config | stderr | reaches preset gate (preset is unknown) — boundary exact |
| control (F1) | head + synthetic presets only | valid instance | MCP handshake + tools/list |
boots, serves exactly context_search |
35/35 scripted assertions (logs/startup-guards.log). The pair proves fail-closed is produced by the empty registry specifically — nothing else gates startup — and that the rest of the pipeline is live, not dead skeleton.
Wire oracle: the bounded engine against real loopback providers
Mock-free: real child process, real stdio JSON-RPC, real node:http providers journaling every request; no fetch stubs anywhere. Witness: 02-wire-post-happy-both-sides.png, 03-wire-redirect-not-followed.png.
| scenario | wire assertions (provider journal) | client assertions | result |
|---|---|---|---|
GET happy (x-api-key) |
1 request; exact path, query/app_id/top_k=5 params; x-api-key header, no authorization |
Profile v1 envelope, NOTICE, memory_id/text mapped |
pass (13) |
POST happy (Token auth) |
1 request; exact path incl. basePath; Authorization: Token …; body deep-equal to contract fixture; no retry |
items mapped (memory→content, updated_at→updatedAt) |
pass (12) |
| 302 → decoy | origin hits = 1, decoy hits = 0 | generic redacted error | pass (4) |
| oversized, declared content-length | — | generic error | pass (2) |
| oversized, streamed 2 MiB | — | generic error | pass (2) |
| 7 valid items | — | capped at 5, provider order preserved | pass (2) |
| HTTP 500 with secret body | — | generic error; no host/token/credential in result | pass (5) |
| malformed JSON / wrong collection | — | generic redacted error each (invalid UTF-8 pinned by the unit suite's fatal-decoder test; same real decode path) | pass (4) |
| query whitespace collapse | provider receives deployment policy |
— | pass (1) |
| whitespace-only query | provider never contacted | rejected | pass (2) |
stall vs timeoutMs: 100 |
request received; socket aborted | prompt redacted error | pass (4) |
<policy> in provider content |
— | model-facing text escaped (\u003cpolicy\u003e), raw absent; structured data preserved |
pass (3) |
| tool surface | model-supplied endpoint arg rejected before any provider contact |
single tool, closed input schema, no endpoint/scope/preset/headers properties | pass (8) |
62/62 (logs/wire-f1.log). One harness-side incident, resolved: the POST cell first failed because my harness config omitted the fixture's basePath — the engine was right, my config was wrong; fixed and re-run green.
Mutation matrix — every guard is load-bearing
Each row reverts one guard (asserted single-edit) in a scratch variant, rebuilds, and asks the package's own suite (vitest JSON reporter) plus the wire/startup oracles. Positive control M0 is caught in the same file as the engine mutants. Witness: 04-mutation-matrix.png.
| mutant | unit suite | wire/startup behavior vs head | classification |
|---|---|---|---|
| F1 preset injection (control) | killed by ships no live provider preset in PR1 | boots (by design) | proves the fail-closed test pins the registry |
| M0 declared-length precheck removed | killed by caps the response before parsing it | wire still errors — undici independently rejects an undelivered declared length (probe: BODY_READ_FAILED terminated) |
pinned at the FetchLike seam; redundant defense at the wire over undici + the streaming cap |
M1 redirect: 'manual' → 'follow' |
killed by 3 tests (redirect test + both contract fixtures pin redirect: 'manual') |
flips: decoy receives the request with x-api-key: "fixture-token"; attacker-controlled body served |
load-bearing — credential exfiltration + content injection via redirect |
| M2 HTTPS gate removed | killed by plain HTTP without opt-in | flips: unopted http:// origin boots |
load-bearing |
M3 engine .slice(0, MAX_RESULTS) removed |
killed by returns at most five valid items | wire holds at 5 (profile MAX_ITEMS still caps) |
layered guard — see combo row |
M3b profile MAX_ITEMS 5→99 |
killed by profile test + shared Profile v1 contract vectors | wire holds at 5 (engine slice still caps) | layered guard — see combo row |
| M3+M3b combination | killed by all three | flips: 7 items served | the set is load-bearing; each single hunk is redundant defense against its sibling — the cap is enforced only by the pair at the model boundary |
M4 % path guard removed |
killed by encoded base-path traversal | flips: boots, and the request goes to /safe/%2e%e/private/memories/search — encoded traversal reaches the wire for the origin server to decode |
load-bearing |
M5 </> escaping removed |
killed by normalizes only the query… | flips: raw <policy> in model-facing text |
load-bearing |
M6 redundant || (status>=300 && <400) clause removed |
survived (37/38 green; 1 red = preset-injection artifact) | wire holds everywhere | redundant defense — !response.ok already rejects 3xx; correct exactly as it stands, nothing to fix |
23/23 expectations met (logs/mutation-matrix.log). The shared contract vectors pin the 5-item cap at the schema level (M3b), so the sibling package's contract participates in holding this package's boundary.
Scaling ladder (untrusted text into parsers)
Query-validation regex, whitespace normalizer, and renderer probed at 2k/3k/5k/20k hostile characters (plain, lone high surrogates, alternating surrogates, alternating whitespace, 20k < content, five heavy items). Every rung ≤ 0.92 ms; no superlinear curve (logs/redos-ladder.log). The bound pattern's three alternatives are mutually exclusive per position, so backtracking is linear by construction — confirmed empirically.
Targeted gates (all liveness-controlled)
| gate | result | liveness control |
|---|---|---|
npm run test -w @qwen-code/external-context-mem0 |
38/38 pass, 5 files | mutation matrix: 8 mutant rows killed by the suite (every guard + the preset-injection control); only the redundant-clause mutant M6 survived, by design |
tsc --noEmit (workspace) |
exit 0 | planted TS2322 in scratch copy → reported; restored → clean |
eslint src (workspace) |
exit 0 | planted unused variable → reported; restored → clean |
npm run build -w … |
single self-contained dist/main.js (1.1 MB); boots standalone with zero env → fail-closed message, exit 1 |
boot test proves no unresolved runtime module |
npm pack --dry-run --json -w … |
exactly: README, dist/main.js, manifest, 2 canonical schemas; no dependencies field; private: true |
content asserted against the PR's claim — matches |
Wiring (secondary claim)
npm ls --workspaces --json --depth=0enumerates 31 workspaces including@qwen-code/external-context-mem0;scripts/version.jsbumps every workspace outside a 3-name exclusion list that does not include it — sorelease:versionbumps this package.- The changed
git addglob, executed under the release step's own shell contract (bash --noprofile --norc,set -euo pipefail), expands to bothintegrations/*/package.jsonfiles; the previous literal path would have left the bumped mem0 version uncommitted — the hunk is load-bearing for the release flow. scripts/build.jsbuild order includes the workspace; the rootnpm run buildat HEAD produced its bundle (this verification consumed it).
Corrections
- Test count. The description says "confirm all 27 synthetic schema, request-engine, manifest, cancellation, redaction, and shared Profile v1 contract tests pass." The suite actually contains 38 tests (manifest 2, profile 2, request-engine 11, schemas 16, mcp 7), all passing. The discrepancy is in the favorable direction, but the number in the description should be updated.
Findings
F1 (Suggestion) — runtime search failures are unobservable to operators
mcp.ts catches every provider failure and returns the generic 'External context search failed.'; nothing is written to stderr or any other channel. An administrator cannot distinguish timeout from credential rejection (401), DNS failure, oversized response, or malformed payload — the value "which failure was this" survives nowhere. The design doc mandates redaction of the model-facing error result ("Error results redact the query, endpoint, credential, upstream response body, and raw exception") — it does not mandate silence toward the operator, and the redaction tests only assert on the tool-result payload, so a stderr line carrying a stable failure category (no endpoint/credential/query/body) would satisfy both. Latent in PR1 (no live preset can search); becomes a real debugging wall once PR2 presets ship. Blast radius: exactly one call site (the tool handler's catch).
Suggested direction (not measured as a patch)
In the context_search handler's catch, write a sanitized category to stderr (timeout / http-status-class / invalid-response / request-failed) before returning the redacted result. This was not implemented/measured in this round — it is a direction for PR2's observability story, and the existing redaction tests place no constraint on stderr.
Informational, not a finding: the (response.status >= 300 && response.status < 400) clause in request-engine.ts is redundant defense (M6 row) — !response.ok already rejects 3xx. Correct as it stands; deleting it would also be fine.
Not covered
- Per-commit attribution. Depth-2 checkout: of the 3 commits in the metadata, only head
0b955721is locally reachable (0ba04d77design doc — which belongs to stacked docs(external-context): Design a configurable Mem0 provider extension #10113 — anddac4ce36skeleton are not). Verified the aggregateHEAD^1..HEADdiff instead. - Repo-wide gates (root build/typecheck/lint,
npm run test:scripts): covered by the PR's own CI; not re-run per budget rules. The author's claim of 2 contention-timeout reruns intest:scriptswas not independently re-verified. No script test imports this package (confirmed by the author; consistent with the diff census). - Live provider behavior (Mem0 Platform V3, PolarDB): intentionally absent in PR1 — proven absent (empty registry, fail-closed), not tested live.
- Extension-manager installation flow (
${extensionPath}substitution, managed install E2E): not exercised; the static manifest contract is pinned by the manifest test and the bundle boots standalone. release:versionexecuted end-to-end: would rewrite workspace versions; instead the two facts it depends on (workspace enumeration, glob expansion) were verified live.- Windows behavior (author marked
⚠️ ): Linux-only verification. - The stacked docs(external-context): Design a configurable Mem0 provider extension #10113 design commit content was reviewed as documentation only; its acceptance belongs to that PR.
Methodology
Environment: node:22 container (Node v22.23.2, npm 10.9.8), merge-ref checkout at depth 2 (HEAD merge commit, HEAD^1 base tip, HEAD^2 verified head). npm ci + root build were pre-run by the workflow. Harnesses (harness/*.mjs, rerunnable) drive the real compiled bundle — produced with the package's own esbuild flags — as a child process over real newline-delimited JSON-RPC stdio, against real loopback node:http providers that journal every request (method, path, headers, body, counts, aborts); MCP drives use a minimal handwritten client, no SDK in the assertion path. Variant builds (harness/variants/) are scratch copies differing from head by exactly one string-asserted edit each (plus the documented preset-injection control); unit suites run per variant via the vitest JSON reporter; variant node_modules symlinks the package's own so nested dependency versions match the shipped bundle. Raw logs in logs/, terminal witnesses in evidence/. One wire cell initially red due to a harness-config bug (missing basePath in my instance config, not the PR's behavior) was diagnosed, fixed, and re-run green; all other cells were green on first execution.
Flakiness gate log
rounds=5 files=5 skipped=0
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/mcp.test.ts: (cd integrations/external-context-mem0) npx --no-install vitest run ./src/mcp.test.ts
file integrations/external-context-mem0/src/profile.test.ts: (cd integrations/external-context-mem0) npx --no-install vitest run ./src/profile.test.ts
file integrations/external-context-mem0/src/request-engine.test.ts: (cd integrations/external-context-mem0) npx --no-install vitest run ./src/request-engine.test.ts
file integrations/external-context-mem0/src/schemas.test.ts: (cd integrations/external-context-mem0) npx --no-install vitest run ./src/schemas.test.ts
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/mcp.test.ts: PPPPP
integrations/external-context-mem0/src/profile.test.ts: PPPPP
integrations/external-context-mem0/src/request-engine.test.ts: PPPPP
integrations/external-context-mem0/src/schemas.test.ts: PPPPP
verdict: pass
summary: 5 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/mcp.test.ts: P (exit 0)
round 1 · integrations/external-context-mem0/src/profile.test.ts: P (exit 0)
round 1 · integrations/external-context-mem0/src/request-engine.test.ts: P (exit 0)
round 1 · integrations/external-context-mem0/src/schemas.test.ts: P (exit 0)
round 2 · integrations/external-context-mem0/src/manifest.test.ts: P (exit 0)
round 2 · integrations/external-context-mem0/src/mcp.test.ts: P (exit 0)
round 2 · integrations/external-context-mem0/src/profile.test.ts: P (exit 0)
round 2 · integrations/external-context-mem0/src/request-engine.test.ts: P (exit 0)
round 2 · integrations/external-context-mem0/src/schemas.test.ts: P (exit 0)
round 3 · integrations/external-context-mem0/src/manifest.test.ts: P (exit 0)
round 3 · integrations/external-context-mem0/src/mcp.test.ts: P (exit 0)
round 3 · integrations/external-context-mem0/src/profile.test.ts: P (exit 0)
round 3 · integrations/external-context-mem0/src/request-engine.test.ts: P (exit 0)
round 3 · integrations/external-context-mem0/src/schemas.test.ts: P (exit 0)
round 4 · integrations/external-context-mem0/src/manifest.test.ts: P (exit 0)
round 4 · integrations/external-context-mem0/src/mcp.test.ts: P (exit 0)
round 4 · integrations/external-context-mem0/src/profile.test.ts: P (exit 0)
round 4 · integrations/external-context-mem0/src/request-engine.test.ts: P (exit 0)
round 4 · integrations/external-context-mem0/src/schemas.test.ts: P (exit 0)
round 5 · integrations/external-context-mem0/src/manifest.test.ts: P (exit 0)
round 5 · integrations/external-context-mem0/src/mcp.test.ts: P (exit 0)
round 5 · integrations/external-context-mem0/src/profile.test.ts: P (exit 0)
round 5 · integrations/external-context-mem0/src/request-engine.test.ts: P (exit 0)
round 5 · integrations/external-context-mem0/src/schemas.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
Follow-up on the sandboxed verification:
No runtime code change is needed for this verification round. |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 85 passed · 0 failed · 85 total Flakiness gate: ✅ 5 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：85 通过 · 0 失败 · 85 总计 抖动门：✅ 5 changed test file(s) x 5 identical rounds, no divergence Verification reportSandboxed verification: ✅ passed — merge-ready (agent verdict, follow-up round) Scripted assertions: 85 passed · 0 failed · 85 total 中文摘要
Previous-round finding status (follow-up)The previous round verified the same head
ScopeCentral claim — the extension is retrieval-only and fails closed: the shipped package (empty built-in preset registry) cannot start as a working provider under any configuration; once a preset is injected at the registry seam the PR itself designates, the bounded request engine enforces the v1 grammar end-to-end over real sockets: HTTPS unless opted in, no redirects, no retries, 1 MiB cap, ≤5 normalized items, redacted generic errors, model surface exactly Secondary claims — (1) self-contained packaging (no runtime deps, exact tarball contents, Profile v1 contract shared with the sibling integration); (2) root wiring valid (workspace registration, build order, release versioning glob) with no new transitive dependencies. The base side of the A/B is an absence control (the package does not exist at A/B: the empty registry is the fail-closed gateWitness:
8/8. F2 is the load-bearing pair with the H2 control: the identical bootstrap with probe presets injected boots and serves (below), so it is the empty registry — and nothing else — that gates startup. Wire oracle: real stdio MCP + real loopback providersThe variant bundle is
Config-boundary ladder (H3)27/27 against the real Mutation matrixWitness:
Corrections
FindingsF1 (Suggestion, carried over, stands) — provider failure classes are unobservable to operators
Minimal fix, measured this round (scratch copy, not applied to the PR)- } catch {
+ } catch (error) {
+ process.stderr.write(
+ `${error instanceof Error ? error.message : 'External context search failed.'}\n`,
+ );
return errorResult('External context search failed.');
}Measured on the patched variant against a 500 provider (h6, 3/3): result text byte-identical and redacted; child stderr now carries Informational, not a finding (agrees with the previous round): the explicit 3xx clause is redundant defense (M3 row). Nothing to fix. Not covered
MethodologyEnvironment: — Qwen Code · sandboxed verification Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run (@doudouOUC) — gate re-checked at Template: complete ✓ Problem: real, established in the first run via #9952 — not re-litigating it. Direction — still waiting on a maintainer decision. What changed since the last run:
Progress acknowledged: the eight review suggestions were addressed in Gate status: the existing Stage 1c What unblocks this: @yiliang114 decides which route is canonical. If the Extension route wins: land #10113 first, then re-run 中文说明Triage 重跑（@doudouOUC）——已在 模板：完整 ✓ 问题：真实存在，首轮已通过 #9952 确认——不再重复论证。 方向——仍在等待维护者决策。 上次运行以来的变化：
已确认的进展：八条 review 建议已在 门禁状态：既有的 Stage 1c 解除条件：@yiliang114 决定哪条路线为 canonical。若 Extension 路线胜出：先合并 #10113，再在此重跑 — Qwen Code · qwen3.8-max Reviewed at |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed at the current head — full pass over the new integrations/external-context-mem0 package plus the infra hooks:
- Request engine: dialect-driven auth (token/bearer/x-api-key) and field placement (query/json/filters/omit),
redirect: manualwith 3xx treated as failure, response bounded both by content-length precheck and streaming byte count, strict UTF-8 decode, and untrusted items filtered (never thrown) before the 5-result cap. This is exactly the shape needed to normalize the per-engine v1/v2 protocol divergence into one validated dialect config. - Config loading: absolute-path-only, 64 KiB bound enforced during the read loop (not after), endpoint restricted to https unless
allowInsecureHttpwith no embedded credentials/query/path in the origin, static-path validation rejecting//, separators, percent, control chars and dot segments beforebuildSearchUrlcomposes anything, GET dialects barred from json placement, and scope value/location consistency enforced.readRequiredEnvironmenteven rejects unsubstituted${VAR}placeholders. - MCP surface: single
context_searchtool, abort + per-instance timeout viaAbortSignal.any, results labelled untrusted reference data, generic error results. - Infra deltas are safe: release.yml widens the version-bump glob to
integrations/*/package.json, workspace + build-order entries added. - Earlier round-1 suggestions all resolved in their threads; CI test lanes green on this head (only routing/label jobs in flight).
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
No blocking findings.
Scope: All source files under integrations/external-context-mem0/src/ (7 files, ~780 lines of reviewed logic), JSON schemas under schemas/, and design doc docs/design/external-context-mem0-extension.md. Not reviewed: test files (titles only).
Checked:
- Credential handling:
applyAuthentication(request-engine.ts:57–73) places all three auth types (authorization-token,authorization-bearer,x-api-key) into HTTP request headers, not URL query strings. No credential exposure in error messages —readRequiredEnvironmentemits only'Mem0 extension configuration is unavailable.' - URL path construction:
buildSearchUrlstrips a trailing slash frombasePathbefore combining withdialect.search.path(config.ts:193:basePath.replace(/\/$/u, '')), preventing the//that independentvalidateStaticPathchecks per-component would miss - Response body bounds:
readBoundedBodyallocates at mostMAX_RESPONSE_BYTES(1 MiB) via streaming byte count cap; content-length precheck rejects declared oversized responses before streaming begins - Config size bound (CI bot R1-2):
Buffer.allocUnsafe(MAX_CONFIG_BYTES + 1)fixes allocation at 64 KiB+1 regardless of file size — the CI bot's claim that it "reads the entire file" was incorrect; the author's fix commit confirms this was already the design validateStaticPath: rejects//,?,#,\,%, control chars, and./..path segments- Schema version gates:
const: 1in bothschemaVersionanddialectVersionfields are closed enums; Ajvstrict: trueenforces them - Round-1 CI bot suggestions (R1-1 through R1-8): all addressed by author in commit
0b9557214c— design doc dialect example now includes all required v1 location fields; test coverage added for: provider timeout,Bearerauth scheme, non-2xx status, invalid UTF-8 body, malformed JSON body,renderResultfield-drop and 4000-char envelope,schemaVersion/dialectVersionnegative contract pins
CI: 0 checks configured — no build, typecheck, lint, or test run is verifiable for this PR. Rungs 1–3 of the execution tier were not performed.
Direction note: The CI bot's CHANGES_REQUESTED is a project-direction question about whether the preset-extension route (this PR, part of design #10113) should supersede the direct polardb integration (#9952). This is a maintainer decision, not a code defect. The implementation itself is clean.
Reviewed with AI assistance.
Already have 2 approves,3ks.
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/release.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/release.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.2)
|
Post-fix summary for Discovered review items:
Fixes and method:
Rejected or deferred:
Verification:
Review threads resolved: 10/10. Decision needed: none. CI for the new head is running; the PR remains Ready and mergeable. |
|
Review feedback batch for f86c1ed
Verification: Prettier check, whitespace check, link-target check, and two clean design-diff audits. Rejected: none. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
中文说明
— qwen3.8-max via Qwen Code /review (v0.22.2)
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
第三轮 re-review（增量：bd085152 → f86c1edf）
增量内容
仅有一个 docs-only 提交（26 行）：
docs/design/external-context-mem0-extension.md的 Non-goals 从「在此提案中实现 Extension」改为「在 PR1 中不包含 live provider presets/adapters/tests」——符合 PR1 已包含骨架代码的实际情况，同时修复了 CI bot R3-1 的建议。- Instance configuration 说明从「conceptual」改为「illustrative」，并标注 PR1 的 canonical schema 为权威来源。
代码逻辑不变
此增量不触及任何代码文件，round 2（bd085152）已 APPROVE 时对 config.ts（whitespace-only credentials 验证）和 profile.ts（five-item limit after filtering）两处 Critical 的验证结论仍然有效。
CI
7 success（含 Test (ubuntu-latest, Node 22.x)、Desktop Shell ubuntu/windows、web-shell E2E Smoke），23 skipped。
无阻塞项。
|
@qwen-code /triage |
|
Sandboxed verification: Skipped because the PR has merge conflicts, so refs/pull/10149/merge is unavailable — resolve conflicts and re-run. 中文 — 判定：
|
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
f86c1ed to
4d2b68a
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
|
Rebased PR1 onto the latest
Rejected/ignored/deferred: the positive re-review, the author-side Decisions needed: none. New head: |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after
Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "extension_local_path_install" |
qwenCodeVersion |
"0.22.0" |
"0.22.2" |
— Qwen Code · serve A/B
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not explored to full depth (tool budget reached): chunk 3: none — I did not run the vitest suite itself (no install/build in the integration package), but verified each assertion statically against the committed impleme….
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/release.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：chunk 3：none — I did not run the vitest suite itself (no install/build in the integration package), but verified each assertion statically against the committed impleme…。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
未检查（工具限制，非阻断）：the executable-script lint — .github/workflows/release.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.2)
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
第四轮 re-review（增量：f86c1edf → 4d2b68aa）
变更性质
纯机械 rebase：PR #10113（设计文档）合入 main 后，本 PR 的 PR0 commit（设计文档）被自动 dropped，其余 4 个 PR1 commit 直接 replay。doudouOUC 在评论中确认：git range-diff 报告每个 patch 等价，49 个 extension 测试、18 个 release-workflow 测试、完整 build/typecheck/lint 均通过。
已核验内容
R2-1（whitespace-only credentials）
readRequiredEnvironment 中 const trimmed = value?.trim() + !trimmed 检查确认仍然保留，whitespace-only 凭据正确被拦截。
R2-2（five-item limit after filtering）
normalizeResponse 的调用链为 .map → .filter → .slice(0, MAX_RESULTS)，先过滤无效条目再截断，与修复意图一致，仍保留。
设计文档变更
本 PR 中的 docs/design/external-context-mem0-extension.md 仅为 23 add/9 del 的状态与措辞对齐（Status 从 Proposed 改为 Partially implemented，Non-goals 更新为 PR1 scope）。
CI
3 success（含 Test ubuntu Node22、Desktop Shell ubuntu/windows），26 skipped，1 in_progress（review-pr）。核心测试套件通过。
CI bot R5-1（Suggestion）
CI bot 对 49 个测试执行了 20 个变异测试，20 个均存活——即测试套件对 20 处保护行为没有覆盖。这是真实的测试覆盖缺口，但属于 Suggestion 级别，不阻塞 PR1（尚无 live preset 走到这些分支）。CI bot 建议在 PR2 落地 live preset 之前补完变异测试。
无阻塞项。
|
Released in v0.22.3. |
5 participants
What this PR does
This is PR1 of the configurable Mem0 provider-extension rollout designed in #10113. It adds a self-contained retrieval-only
external-context-mem0stdio Extension, canonical versioned schemas for administrator-owned instance configuration and closed dialect presets, a bounded HTTP request engine, and contract tests against synthetic GET and POST providers. The model-facing surface remains exactlycontext_search({ query }); endpoint, credential environment variable, scope, timeout, and preset selection are fixed at startup.The request engine supports only the allowlisted v1 grammar, requires HTTPS unless plain HTTP is explicitly enabled, rejects unsafe endpoint paths, does not follow redirects or retry requests, caps provider responses at 1 MiB, normalizes at most five results, and returns them through External Context MCP Profile v1 as untrusted reference data. The package is wired into workspace build and release versioning as a self-contained bundle.
PR1 intentionally ships an empty built-in preset registry and therefore does not enable a live provider. Verified Mem0 Platform V3 and PolarDB MySQL presets remain PR2 work.
Why it's needed
The current direct provider integration cannot scale to Mem0-compatible products with different but bounded REST contracts without adding another provider-specific branch to Qwen Core. This package establishes the independently versioned Extension boundary from #10113 so later compatible services can be added as reviewed preset data while endpoint and credential ownership stay outside the model contract.
Reviewer Test Plan
How to verify
Run
npm run test --workspace=@qwen-code/external-context-mem0and confirm all 49 synthetic schema, request-engine, manifest, cancellation, redaction, and shared Profile v1 contract tests pass. Run the package typecheck, lint, build, andnpm pack --dry-run --json --workspace=@qwen-code/external-context-mem0; confirm the archive contains only the bundled server, manifest, README, and canonical schemas, with no live preset or runtime dependency installation. Run the root build, typecheck, and lint to confirm the workspace and release integration remains valid.Local
npm run test:scriptscompleted 64 of 66 files and 1690 tests; two unrelated workflow tests exceeded their suite-level 20/30-second timeouts under contention, and both passed when rerun individually in 8.44 seconds and 10.22 seconds. No script test exercises or imports this new package.Evidence (Before & After)
N/A — this is a non-UI Extension package and does not change existing runtime behavior because no live preset is enabled.
Tested on
Environment (optional)
macOS 26.4.1 arm64, Node.js v26.0.0, npm 11.12.1. Provider tests use local synthetic fixtures and in-memory MCP transports only.
Risk & Scope
Linked Issues
Depends on #10113. Related to #9952.
This PR is stacked on commit
0ba04d771b72499e94ad4cc8cf40d0c32222c1a8from #10113. Until #10113 merges, GitHub shows that design commit in this PR's comparison againstmain; after it merges, this PR reduces to the PR1 implementation commit.中文说明
本 PR 做了什么
这是 #10113 所设计的可配置 Mem0 Provider Extension 分阶段方案中的 PR1。它新增一个自包含、只读检索的
external-context-mem0stdio Extension，提供管理员持有的实例配置与封闭 dialect preset 的 canonical 版本化 schema、受限 HTTP 请求引擎，以及基于合成 GET/POST provider 的契约测试。面向模型的接口严格保持为context_search({ query })；endpoint、凭证环境变量、scope、timeout 和 preset 均在启动时固定。请求引擎仅支持 allowlist 中的 v1 语法；默认要求 HTTPS，只有显式启用时才接受 HTTP；拒绝不安全 endpoint 路径；不跟随重定向、不重试；provider 响应上限为 1 MiB；最多规范化五条结果；并通过 External Context MCP Profile v1 将结果标记为不可信参考数据返回。该 package 已接入 workspace 构建和 release 版本更新，并产出自包含 bundle。
PR1 有意提供空的内置 preset registry，因此不会启用任何真实 provider。经过验证的 Mem0 Platform V3 和 PolarDB MySQL preset 仍属于 PR2。
为什么需要
当前直接 provider 集成无法在不继续向 Qwen Core 添加 provider-specific 分支的情况下扩展到 REST 契约不同但仍可受限描述的 Mem0 兼容产品。本 package 落实 #10113 中独立版本化的 Extension 边界，使后续兼容服务可以通过经过评审的 preset 数据接入，同时 endpoint 和凭证所有权始终位于模型契约之外。
Reviewer 测试计划
如何验证
运行
npm run test --workspace=@qwen-code/external-context-mem0，确认 49 个关于合成 schema、请求引擎、manifest、取消、脱敏和共享 Profile v1 契约的测试全部通过。运行 package 的 typecheck、lint、build 以及npm pack --dry-run --json --workspace=@qwen-code/external-context-mem0；确认归档只包含 bundled server、manifest、README 和 canonical schema，不包含 live preset，也不需要安装运行时依赖。运行根目录 build、typecheck 和 lint，确认 workspace 与 release 集成有效。本地
npm run test:scripts完成 66 个文件中的 64 个及 1690 个测试；两个与本 PR 无关的 workflow 测试在整套测试资源竞争下超过各自 20/30 秒 timeout，单独重跑分别在 8.44 秒和 10.22 秒通过。没有 script test 引用或导入此新 package。证据（Before & After）
N/A——这是非 UI Extension package；由于未启用 live preset，它不会改变现有运行时行为。
已测试平台
环境（可选）
macOS 26.4.1 arm64、Node.js v26.0.0、npm 11.12.1。Provider 测试只使用本地合成 fixture 和内存 MCP transport。
风险与范围
关联事项
依赖 #10113，与 #9952 相关。
本 PR 堆叠在 #10113 的提交
0ba04d771b72499e94ad4cc8cf40d0c32222c1a8之上。在 #10113 合并前，GitHub 相对main的比较会包含该设计提交；合并后，本 PR 将只剩 PR1 实现提交。