Fix #2212: Bug: viewer config save overwrites viewer.port with UI default (18799) on Hermes - #2219
Conversation
…PATCH Sanitize incoming PATCH /api/v1/config bodies inside core/config/writer.ts so viewer.port and viewer.bindHost — owned at runtime by the Hermes/OpenClaw adapters via bridge.mts AGENT_DEFAULT_PORTS — are silently stripped before the deep-merge writes them to disk. Also drop empty-string patches on embedding/llm/l3Llm/skillEvolver endpoint fields to prevent the UI's rehydrated placeholder from wiping a previously-configured endpoint. Fixes MemTensor#2212: on Hermes, saving from the Memory Viewer used to clobber viewer.port with the UI default 18799 (the OpenClaw port), breaking the bridge until config.yaml was hand-edited back to 18800. - Added sanitizePatch() helper with ADAPTER_OWNED_PATCH_PATHS and NON_EMPTY_PATCH_PATHS whitelists; prunes now-empty parent maps. - 4 new tests in tests/unit/config/writer.test.ts covering both guards plus the openOnFirstTurn-still-patchable invariant. - Updated existing schema-validation test to use bridge.port since viewer.port now bypasses validation via strip. Test evidence: 11/11 writer tests pass, 1265/1265 unit tests pass, tsc --noEmit clean.
Memtensor-AI
added
ai:generated
🤖 Open Code ReviewTarget: PR #2219 ✅ OpenCodeReview: No comments generated. Looks good to me. Generated by cloud-assistant via Open Code Review. |
🔧 Open Code Review requested Agent fixOpen Code Review found 2 issue(s). I have resumed the development Agent to fix them.
The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed. |
Address two Open Code Review findings on the MemTensor#2212 fix: - writer.ts:171 — replace `JSON.parse(JSON.stringify(patch))` with `structuredClone(patch)`. The JSON round-trip silently drops `undefined` leaves, so a caller passing `{ llm: { endpoint: undefined } }` would have that key vanish before `applyPatch` could act on it — a silent no-op instead of a visible error. structuredClone is available on Node 17+ (package engines require >=20) and preserves the full object graph. - writer.ts:177 — trim before comparing the endpoint guard so whitespace-only strings (`" "`, `"\t\n"`) submitted by UI forms are dropped the same way exact `""` is. Also adds a `typeof` guard so a non-string value at one of these paths cannot crash the check. Mirrors the treatment used by `stripEmptySecrets` on secret fields. Test evidence: - 3 new tests in tests/unit/config/writer.test.ts (whitespace guard, structured-clone semantics — via schema-rejection assertion — and the previously-added empty-string guard remains green). - 13/13 writer tests pass. - 1267/1267 unit tests pass, tsc --noEmit clean.
✅ Automated Test Results: PASSEDAll tests passed (20/20 executed). memos_local_plugin/unit: 11/11, memos_python_core/changed-repo-python: 9/9. Duration: 8s [advisory, non-gating] AI-generated tests on branch test/auto-gen-24db7cc632d778ac-20260805145601: 61/63 passed, 2 failed — these do NOT affect the PR verdict; review the branch manually. Branch: |
Memtensor-AI
added
status:ready
|
Synced this branch with the current The sanitizer had classified two user-owned values as immutable/non-empty:
The fixed Regression evidence:
The PR is conflict-free and currently |
3 participants
Description
Fix #2212 by adding a
sanitizePatch()guard insideapps/memos-local-plugin/core/config/writer.ts. The writer now silently strips adapter-owned viewer keys (viewer.port,viewer.bindHost) from every incomingPATCH /api/v1/configbody before the deep-merge writes to disk, so the Hermes adapter's hardcoded:18800viewer port can no longer be clobbered by the UI's default:18799(the OpenClaw port). The same helper drops empty-string patches onembedding.endpoint,llm.endpoint,l3Llm.endpoint, andskillEvolver.endpoint, matching the defensive treatment already applied to secret fields upstream and closing the companion "placeholder text overwrote endpoint" symptom from the same report.Root cause:
writer.ts::patchConfigdeep-merged the client patch verbatim (only stripping empty secrets andembedding.dimensions), with no guard for keys the adapter owns at runtime.bridge.mts::AGENT_DEFAULT_PORTShardcodeshermes → 18800, so any PATCH carrying a differentviewer.portcorruptedconfig.yamland broke bridge/stdio linkage until the file was hand-repaired. Fix lives at the single choke point every PATCH path traverses (routes, direct core calls, hub-triggered rewrites), so no callers need to change.Test evidence — all real pytest/vitest output:
tests/unit/config/writer.test.ts(viewer.port strip, viewer.bindHost strip,openOnFirstTurnstill patchable,embedding.endpointempty-string protection); confirmed all 4 failed before fix, all 11 tests in the file pass after.paths.test.ts,load.test.ts,writer.test.ts.tests/unit/server/http.test.ts— includes the Bug: /api/v1/embeddings/maintenance causes 100% CPU and event loop starvation on large corpora #1929 400-vs-500 PATCH contract): 77/77 pass.tsc -p tsconfig.json --noEmitclean.One existing test (
validates after merge — invalid patches are rejected) was retargeted fromviewer.port: -3tobridge.port: -3becauseviewer.portnow bypasses schema validation via the new strip — noted inline. Task archive synced to memos-autodev-specs main.Related Issue (Required): Fixes #2212
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Automated tests are pending.
Checklist
@whipser030, @hijzy please review this PR.
Reviewer Checklist