[v3.8.50] feat(memory): MemoryBackend provider pattern with generic HTTP connector - #8752
diegosouzapw merged 13 commits into
Conversation
84ca591 to
42f9c11
Compare
⏸️ Ejetada do merge-train v3.8.49 — triagem de cherrypick pendenteEsta PR foi ejetada do merge-train local (30→24 PRs) porque sua pegada no diff excede o orçamento seguro de validação em lote (>700 linhas ou nova fronteira arquitetural). Não é rejeição — é adiamento com motivo rastreável. Motivo da ejeção
Como destravar (sugestão do operador)O operador pediu que toda PR ejetada seja revisada para possível cherrypick. Providências sugeridas por categoria:
Próximos passos
|
Merge-train ejection — status updateThanks for the detailed breakdown of why this PR was ejected. Here is what has been done: Fixed:
Pre-existing (not from our changes):
Plan:Per your suggestion, I can prepare a cherry-pick subset isolating the most impactful pieces:
Happy to arrange a 1-on-1 architecture review if you prefer to validate the full module boundary first. Which path works best? |
|
Re-homed to |
f84efe4 to
6c92b89
Compare
- MemoryBackend interface + MemoryManager singleton - SQLiteBackend (thin wrapper, zero behavior change) - GenericMemoryBackend with dynamic endpoint/path/query mapping - ObsidianBackend (markdown + frontmatter) - Settings: primaryBackend, fallbackBackends, backendConfigs - API routes delegated to MemoryManager - KNOWN_BACKENDS presets: Vilona, Obsidian, Notion
- Remove Vilona from KNOWN_BACKENDS presets - Remove Vilona references from PR body and code comments - Keep Obsidian and Notion as generic examples
- Add MEMORY_VEC_TOP_K, MEMORY_RRF_K to Memory Engine section - Add NOTION_API_KEY, NOTION_API_URL, OBSIDIAN_API_KEY, OBSIDIAN_API_URL - Fix .env.example sync for new memory backend vars
…y migration must have valid SQL)
…nit tests - Fix get/update/delete to pass both pathParams.id and memoryId - Fix buildListQuery to not filter offset=0 as falsy - Add 26 comprehensive unit tests for GenericMemoryBackend (health, init, CRUD, search, auth headers, factory)
…est-discovery collector Ensures generic-backend.test.ts is collected by CI runner (vitest) instead of being detected as a new orphan by the test-discovery gate.
…ndex.ts + add MEMORY_BACKEND docs
6c92b89 to
511614a
Compare
…p, add SSRF guard, remove .skip - Remove PR_BODY.md artifact (committed by mistake) - Wire initMemoryBackends() into instrumentation-node.ts startup - Remove .skip from retrieval.test.ts FTS5 test describe block - Add SSRF prevention guard in GenericMemoryBackend request() method (blocks loopback, private, cloud-metadata IPs and non-http schemes) - Add 9 unit tests for SSRF validation Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
summarization.ts declared its own local MemoryRow interface with type: string, unlike store.ts's MemoryRow (type: MemoryType). Both read the same `memories.type` column, and rowToMemory() already casts the value to MemoryType downstream, so the looser interface type was untightened debt rather than an intentional difference. Narrows the field to MemoryType, matching store.ts's convention. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
2ddbbc6
into
diegosouzapw:release/v3.8.50
…e — every handler 500'd (#9737) (#9785) * fix(api): validate request bodies with Zod in 4 routes — restores the t06 gate The release-green verdict (#9737) lists check:route-validation:t06 as a HARD failure and it is STILL red on the current tip: four routes call request.json() and hand-roll `typeof x === "string"` checks instead of using Zod, which Hard Rule #7 requires and the gate enforces (it scans source and has no allowlist). - src/app/api/plugins/marketplace/install (#9445): InstallBodySchema; the 400 'Missing or invalid name field' response is preserved verbatim. - src/app/api/services/dario/admin/accounts (#8523): DeleteAccountBodySchema for the optional { alias } DELETE body; query-param path untouched. - src/app/api/services/dario/admin/login-start (#8523): LoginStartBodySchema; trimming now happens in the schema, so the forward body is unchanged. - src/app/api/services/dario/admin/import-from-omniroute (#8523): ImportBodySchema for connectionId/alias; invalid shapes fall back to the same 'connectionId is required' 400 as before. All four keep their exact status codes and messages — this is a validation mechanism swap, not a contract change (plugins route suite still 33/33). Adds tests/unit/route-body-validation-t06.test.ts, which runs the gate's own rule inside the unit suite so the next such route fails on ITS OWN PR instead of surfacing weeks later in a base-red sweep. Guard verified by mutation: renaming .safeParse( in one route makes it fail (1 fail), restored from a pre-probe copy. Gates: route-validation:t06, file-size, test-discovery, mutation-test-coverage, dead-code exit 0; typecheck:core clean; eslint clean. Refs #9737 * fix(memory): register the sqlite backend on the /api/memory/[id] route — every handler 500'd GET/PUT/DELETE /api/memory/[id] threw `Primary backend "sqlite" not registered` and returned 500. #8752 (MemoryBackend provider pattern) wired the route to `@/lib/memory/manager` directly, but the registry is populated by an import-time side effect in the module INDEX (src/lib/memory/index.ts:23, `memoryManager.register(sqliteBackend)`). Importing the bare manager gives an empty registry. In production the failure is order-dependent, which is why it went unnoticed: if /api/memory (which imports the index) is hit first in the same process, the singleton is already populated and [id] works. Reached first — the common case for a client that edits a known memory id — every request 500s. The sibling route is the only other consumer and already imports the index; this was the lone direct-manager import in src/. - Fix: import from `@/lib/memory` (index) with a comment stating WHY the indirection matters, so the next refactor does not simplify it back. - Guard: tests/integration/memory-route-put.test.ts already covered this and was failing 2/5 on the base (it only surfaced now because the integration suite runs on the release-PR CI, not per-PR). Now 5/5. Also fixes a test-isolation defect in the same run: tests/integration/combo-matrix/context-relay-codex.test.ts reused one combo name across both tests, and the control failed with `UNIQUE constraint failed: combos.name` — resetStorage() unlinks the DB file but the previous better-sqlite3 handle keeps writing to the same inode. Gave the control its own combo name and parameterized the request builder; the assertion is unchanged (it never depended on the name). 2/2. Integration suite on this tip: 936 tests, 32m19s — under the 40min ceiling the old verdict reported as exceeded (#9737 item 6), which the migration-135 collision was causing. Refs #9737 --------- Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
…TTP connector (diegosouzapw#8752) Validated in local merge-train T7 (ungrouped batch 2)
…e — every handler 500'd (diegosouzapw#9737) (diegosouzapw#9785) * fix(api): validate request bodies with Zod in 4 routes — restores the t06 gate The release-green verdict (diegosouzapw#9737) lists check:route-validation:t06 as a HARD failure and it is STILL red on the current tip: four routes call request.json() and hand-roll `typeof x === "string"` checks instead of using Zod, which Hard Rule diegosouzapw#7 requires and the gate enforces (it scans source and has no allowlist). - src/app/api/plugins/marketplace/install (diegosouzapw#9445): InstallBodySchema; the 400 'Missing or invalid name field' response is preserved verbatim. - src/app/api/services/dario/admin/accounts (diegosouzapw#8523): DeleteAccountBodySchema for the optional { alias } DELETE body; query-param path untouched. - src/app/api/services/dario/admin/login-start (diegosouzapw#8523): LoginStartBodySchema; trimming now happens in the schema, so the forward body is unchanged. - src/app/api/services/dario/admin/import-from-omniroute (diegosouzapw#8523): ImportBodySchema for connectionId/alias; invalid shapes fall back to the same 'connectionId is required' 400 as before. All four keep their exact status codes and messages — this is a validation mechanism swap, not a contract change (plugins route suite still 33/33). Adds tests/unit/route-body-validation-t06.test.ts, which runs the gate's own rule inside the unit suite so the next such route fails on ITS OWN PR instead of surfacing weeks later in a base-red sweep. Guard verified by mutation: renaming .safeParse( in one route makes it fail (1 fail), restored from a pre-probe copy. Gates: route-validation:t06, file-size, test-discovery, mutation-test-coverage, dead-code exit 0; typecheck:core clean; eslint clean. Refs diegosouzapw#9737 * fix(memory): register the sqlite backend on the /api/memory/[id] route — every handler 500'd GET/PUT/DELETE /api/memory/[id] threw `Primary backend "sqlite" not registered` and returned 500. diegosouzapw#8752 (MemoryBackend provider pattern) wired the route to `@/lib/memory/manager` directly, but the registry is populated by an import-time side effect in the module INDEX (src/lib/memory/index.ts:23, `memoryManager.register(sqliteBackend)`). Importing the bare manager gives an empty registry. In production the failure is order-dependent, which is why it went unnoticed: if /api/memory (which imports the index) is hit first in the same process, the singleton is already populated and [id] works. Reached first — the common case for a client that edits a known memory id — every request 500s. The sibling route is the only other consumer and already imports the index; this was the lone direct-manager import in src/. - Fix: import from `@/lib/memory` (index) with a comment stating WHY the indirection matters, so the next refactor does not simplify it back. - Guard: tests/integration/memory-route-put.test.ts already covered this and was failing 2/5 on the base (it only surfaced now because the integration suite runs on the release-PR CI, not per-PR). Now 5/5. Also fixes a test-isolation defect in the same run: tests/integration/combo-matrix/context-relay-codex.test.ts reused one combo name across both tests, and the control failed with `UNIQUE constraint failed: combos.name` — resetStorage() unlinks the DB file but the previous better-sqlite3 handle keeps writing to the same inode. Gave the control its own combo name and parameterized the request builder; the assertion is unchanged (it never depended on the name). 2/2. Integration suite on this tip: 936 tests, 32m19s — under the 40min ceiling the old verdict reported as exceeded (diegosouzapw#9737 item 6), which the migration-135 collision was causing. Refs diegosouzapw#9737 --------- Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
feat(memory): MemoryBackend Provider Pattern with Generic HTTP Connector
Summary
Introduces a pluggable MemoryBackend provider architecture with a generic HTTP connector that supports dynamic endpoint/query/path mapping for any REST-based memory backend (Obsidian, Notion, custom).
Changes
Core Architecture (Phases 1-3)
src/lib/memory/backend.tsMemoryBackendinterface +CreateMemoryInput,MemoryFilter,SearchConfig,HealthCheckResultsrc/lib/memory/manager.tsMemoryManagersingleton — register, configure, fallback routing, health checkssrc/lib/memory/sqliteBackend.tsstore.ts/retrieval.ts— implementsMemoryBackendsrc/lib/memory/index.tsinitMemoryBackends()for app bootstrapsrc/app/api/memory/route.tsmemoryManager.list()/memoryManager.create()src/app/api/memory/[id]/route.tsmemoryManager.get/delete/update()Optional Backends (Phase 4)
src/lib/memory/genericBackend.tssrc/lib/memory/obsidianBackend.tssrc/lib/memory/settings.tsprimaryBackend,fallbackBackends,backendConfigs+ normalizationsrc/shared/schemas/memory.tsMemorySettingsExtendedSchemaSettings Schema (DB + API)
{ "memoryEnabled": true, "primaryBackend": "sqlite", "fallbackBackends": ["obsidian"], "backendConfigs": { "obsidian": { "baseUrl": "http://localhost:27123", "apiKey": "...", "endpoints": { "search": "/api/v1/vault/memories/search", "create": "/api/v1/vault/memories", "get": "/api/v1/vault/memories/{memoryId}" }, "queryParams": { "query": "q", "apiKeyId": "api_key" }, "pathParams": { "id": "memoryId" } } } }GenericMemoryBackend — Dynamic Endpoint Mapping
Supported mappings:
endpoints— override any REST path (supports{id},{memoryId}placeholders)queryParams— rename any query parameter (query→q,apiKeyId→api_key, etc.)pathParams— rename path placeholders (id→memoryId)Connecting Global
omnirouteto Custom Memory BackendVerification
tsc --noEmit— cleanstore.ts/retrieval.tsidentical to v3.8.49 (newline-only diff)Migration Notes
store.ts/retrieval.tsunchangedprimaryBackend: "sqlite",fallbackBackends: [])chatCore.ts, dashboard, CLI) work unchanged — they route throughmemoryManagerFuture Work (Not in this PR)
omniroute-memory-backend-*)