Add external memory approval queue UI

[kiosvantra]

Summary

Adds a generic External Memory Approval Queue surface to the WebUI Memory panel.

This is intentionally provider-neutral. WebUI ships no provider-specific backend, endpoint, hostname, model, collection, or built-in memory provider. Users opt in by registering a custom SQLite-backed approval queue in external_memory_providers.json.

Tracking issue: #1980

What changed

• Adds api/external_memory.py as a generic backend helper for external memory approval queues.
• Adds /api/external-memory/* routes for:
  • provider discovery
  • candidate listing
  • candidate search
  • edit before approval
  • approve / reject / delete actions
• Adds an External Memory section under the existing Memory panel.
• Shows a clear empty state when no providers are configured.
• Keeps approval fail-closed: if indexing config is missing or indexing fails, the candidate remains unchanged.
• Keeps existing My Notes / User Profile memory editing unchanged.

Provider contract

A provider can appear in the UI when it is explicitly registered from the active Hermes home:

{
  "providers": [
    {
      "id": "custom_store",
      "label": "Custom Store",
      "db_path": "custom_memory/items.sqlite",
      "config_path": "custom_memory/config.json"
    }
  ]
}

The provider database exposes this SQLite table:

candidates(
  id text primary key,
  text text not null,
  source text not null default 'agent',
  metadata_json text not null default '{}',
  state text not null default 'candidate',
  content_sha256 text not null,
  created_at real not null,
  updated_at real not null
)

Optional indexing settings must come from provider config or environment variables. If they are absent, approval returns a clear not-configured error instead of using defaults.

Tests

python -m pytest tests/test_external_memory_api.py tests/test_external_memory_review_ui.py -q
# 25 passed

[nesquena-hermes]

Review — substantive, but needs rework before merge

Thanks @kiosvantra — the contract design (provider-oriented review surface, SQLite candidates table shape, optional config registration via external_memory_providers.json) is well thought out, and the 20 passing tests demonstrate the implementation works. Adding maintainer-review while a few significant concerns get addressed.

🔴 Blocker 1: hardcoded private IP addresses leak into a public repo

api/external_memory.py bakes contributor-internal infrastructure into the source tree:

DEFAULT_OLLAMA_URL = "http://10.0.100.50:11434"
DEFAULT_QDRANT_URL = "http://10.0.100.16:6333"

These are RFC1918 private addresses (10.0.100.0/24) that point at someone's specific internal network. nesquena/hermes-webui is a public repo — once these land on master, they're permanent in the git history and grep-able by anyone scanning the codebase for environment leaks.

Project policy is no IPs, no internal hostnames, no org-specific addresses anywhere in the public source tree. Even baked-in as "defaults" they're a leak: a user who never configures a custom value would have the WebUI silently attempt to connect to your internal Ollama/Qdrant instances on first use, and the addresses are now public knowledge.

Required change: remove these constants entirely. Connection URLs must come exclusively from one of:

1. external_memory_providers.json (per-provider config, the path you already define)
2. Environment variables (e.g. HERMES_EXTERNAL_MEMORY_OLLAMA_URL)
3. The provider's own config_path JSON file (which the spec already supports)

When no URL is configured, the provider should disable itself with a clear "not configured" UI state rather than fall back to a hardcoded default that points anywhere.

🔴 Blocker 2: "HMS Knowledge" baked in as a built-in provider

_builtin_provider_specs() hardcodes:

ProviderSpec(
    id="hms_knowledge",
    label="HMS Knowledge",
    kind="builtin",
    db_path=home / "hms_knowledge" / "knowledge.sqlite",
    config_path=home / "hms_knowledge.json",
)

Plus DEFAULT_COLLECTION = "hms_eml_v0_1_memories" and DEFAULT_EMBED_MODEL = "bge-m3:latest" as the implicit defaults.

"HMS Knowledge" appears nowhere else in nesquena/hermes-webui or hermes-agent — verified by grep -rn "hms_knowledge\|HMS Knowledge" across both repos. It's a project-specific provider that doesn't belong as a built-in default for a generic feature.

Required change: drop the _builtin_provider_specs list to empty. Every provider should be a custom registration — let users define their own via external_memory_providers.json. The PR description already documents this path, so the codebase should match.

If "HMS Knowledge" is genuinely meant to be a generic well-known provider that ships with the WebUI, it needs (a) public documentation explaining what it is, (b) a public reference implementation, (c) no internal-IP defaults, and (d) a separate PR proposing the general "built-in providers" concept for review.

🟡 Concern 3: scope is large for a feature with no prior issue

+1100 LOC adding a new top-level UI section, a new api/external_memory.py module, six new routes, and 273 lines of test coverage. There's no referenced GitHub issue describing the user-facing motivation, the design discussion, or the scope agreement.

For features at this scope, the project flow is typically:

1. File an issue describing the problem you're trying to solve and the proposed shape (e.g. "Memory panel should support reviewing external candidate memories from arbitrary providers; here's a contract proposal...").
2. Get a maintainer ack on the rough shape.
3. Then PR the implementation, referencing the issue.

This avoids the contributor spending 1000+ LOC on something the maintainer might shape differently or want narrower. It also gives users a place to find context after the PR ships ("what is this provider thing for?").

Could you file an issue first describing the use case + contract design? I'm happy to discuss the API shape there before the implementation lands. The current implementation can be referenced in the issue so the conversation is grounded in concrete code.

🟡 Concern 4: the contract overlap with existing memory infrastructure

Hermes already has nine memory plugins under ~/.hermes/hermes-agent/plugins/memory/ (mem0, supermemory, byterover, hindsight, holographic, honcho, openviking, retaindb). They each implement a memory store with their own config + connection logic.

This PR's ExternalMemoryProvider contract is a review-and-approval workflow, which is conceptually different from those memory plugins (which write/read on the agent's behalf without human-in-the-loop review). But the naming and surface area overlap: a user looking at the new "External Memory" panel section might reasonably expect it to surface candidates from any of those nine plugins, when in fact it only surfaces candidates from a separate SQLite store the user has to set up themselves.

Worth either:

• Renaming to something more specific (e.g. "Memory Candidates Review" or "External Memory Approval Queue") to set the right expectation
• Or documenting clearly in the panel UI what the relationship is to existing memory plugins (does any of them write to this store? do they need to?)

🟢 What's solid

The actual contract design is good:

• The ProviderSpec dataclass + to_dict() is a clean contract surface
• _safe_provider_id() validation prevents path traversal in db_path resolution — correct choice
• VALID_CANDIDATE_STATES = {"candidate", "approved", "rejected"} is the right enum scope
• The "approval fail-closed" semantics (indexing must succeed before marking approved) is the right invariant for a human-in-loop review queue
• 20 passing tests covering API + UI surfaces — solid test density
• The external_memory_providers.json registration path is genuinely useful for letting users plug in their own SQLite review stores

Once the IP/HMS/scope concerns are resolved, the architectural bones are good.

Suggested path forward

1. File a tracking issue describing the user-facing problem this solves and the contract design. Reference this PR as the candidate implementation.
2. Open a v2 PR that:
   • Removes DEFAULT_OLLAMA_URL, DEFAULT_QDRANT_URL, DEFAULT_COLLECTION, DEFAULT_EMBED_MODEL as hardcoded constants
   • Drops _builtin_provider_specs() to an empty list (no built-ins ship)
   • Provides a clear "no providers configured yet — see docs to register one" empty state in the UI
   • Adds a docs page (or docs/external-memory.md) showing how to register a custom provider via external_memory_providers.json
   • References the new tracking issue
3. The 20 existing tests stay — they're good coverage; just update them to use synthetic fixture URLs/IDs rather than the leaked defaults.

Closing this PR is not the right call — the work is mostly fine. But it can't merge with the current defaults baked in. Please refile with the architectural fixes and I'll prioritize the v2 review.

Thanks again for the PR — happy to help shape the v2 in the tracking issue once you file it.

[kiosvantra]

Thanks for the detailed review — I addressed the blockers and rewrote the PR branch history.

Changes made:

• Force-pushed the branch to a single sanitized commit based on current origin/master.
• Removed all provider-specific built-ins from the implementation.
• Removed all hardcoded connection/model/collection defaults; indexing config now comes only from provider config or environment variables.
• Added the no-providers empty state in the UI.
• Approval now fails closed with a clear not-configured error when indexing config is absent.
• Updated tests and docs to use only synthetic custom provider fixtures.
• Filed tracking issue Support provider-neutral external memory approval queues #1980 for the API/UX shape discussion.

Local verification:

python -m pytest tests/test_external_memory_api.py tests/test_external_memory_review_ui.py -q
# 25 passed

I also scanned the PR diff and updated PR body for the removed provider-specific/internal references. Since the earlier review comment quotes removed implementation details, could you please edit or minimize that comment from the public PR thread if appropriate?

[nesquena-hermes]

Bumping — last review pass landed May 9, blockers were addressed and the branch was force-pushed to a single sanitized commit May 10. CI is currently UNSTABLE; can you triage the failing checks so we can move this from need-updates back into review?

Specifically, please drop a comment confirming:

1. Which CI jobs are red and what's failing (or post the run URL).
2. Whether the force-push picked up any new ruff/pytest issues introduced by the rebase.

Once CI is green and the need-updates blockers have been addressed in commits (not just discussion), I'll remove hold + need-updates and route through the next sweep.

[kiosvantra]

Thanks for the bump.

I checked both the PR and the linked design issue to make sure I was looking at the right place. The design discussion on #1980 looks separate from this CI question; the unstable signal appears to be on this PR branch.

From what I can see, the latest workflow run for this branch did not actually execute any jobs. It completed with action_required, and GitHub is not reporting any red job names or logs:

gh pr checks 1975 --repo nesquena/hermes-webui --watch=false
# no checks reported on the 'external-memory-providers-panel-clean' branch

gh run view 25636760675 --repo nesquena/hermes-webui --json conclusion,jobs,url
# conclusion: action_required
# jobs: []
# url: https://github.com/nesquena/hermes-webui/actions/runs/25636760675

So I do not currently see a failing ruff/pytest job to triage. This looks like the workflow is waiting for maintainer approval to run, rather than a test failure from the force-push/rebase.

The branch currently contains the sanitized implementation updates for the earlier blockers:

• no provider-specific built-ins
• no hardcoded connection/model/collection defaults
• explicit custom provider registration only
• no-providers empty state
• fail-closed approval when indexing config is missing or indexing fails
• synthetic fixtures in tests/docs
• tracking issue Support provider-neutral external memory approval queues #1980 linked for the API/UX design discussion

If you approve/rerun the workflow and a concrete job goes red, I’ll triage that run directly.

[kiosvantra]

Thanks — I found the CI failure and pushed a small fix.

The failing job was:

Tests / test (3.11)
https://github.com/nesquena/hermes-webui/actions/runs/25636760675/job/75472204355

It was failing because the External Memory review actions still used browser-native prompt() in static/panels.js, which tripped the existing static UI tests:

tests/test_kanban_ui_static.py::test_kanban_dashboard_parity_core_controls_are_native
tests/test_sprint33.py::test_no_native_prompt_calls_remain_in_static_js

Fix pushed in 3e34cdf:

• Replaced the edit-candidate native prompt with showPromptDialog().
• Replaced the reject-reason native prompt with showPromptDialog().
• Preserved cancel behavior so no API call is made when the dialog is dismissed.

Focused local verification:

uvx --from pytest pytest \
  tests/test_sprint33.py::test_no_native_prompt_calls_remain_in_static_js \
  tests/test_kanban_ui_static.py::test_kanban_dashboard_parity_core_controls_are_native \
  -q
# 2 passed

uvx --from pytest pytest \
  tests/test_external_memory_api.py \
  tests/test_external_memory_review_ui.py \
  tests/test_sprint33.py \
  tests/test_kanban_ui_static.py \
  -q
# 66 passed

The new workflow run for the pushed commit is back in action_required with no jobs scheduled yet:

https://github.com/nesquena/hermes-webui/actions/runs/25748108240
head: 3e34cdfec25b1e0e0c243f3e43058fd19d7a6eaa
conclusion: action_required

Once that workflow is approved/rerun, I’ll triage any remaining concrete failures if they appear.

[nesquena-hermes]

Removing hold and need-updates labels — ready for review

CI is green now after the prompt() fix you pushed May 12. The static UI tests (test_kanban_ui_static.py::test_kanban_dashboard_parity_core_controls_are_native + test_sprint29.py::test_no_native_prompt_call) that were blocking the merge are passing.

Branch is CLEAN/MERGEABLE. Maintainer-review label is kept since this is a 1377-LOC architectural UI feature that needs careful review. Re-queuing for the next sweep.

[nesquena-hermes]

Closing this PR — recommending it move upstream to hermes-agent instead.

Thanks @kiosvantra for the substantial work here, and to @IkaRiche on #1980 for the KiLu validation offer. After end-to-end re-review (code is clean, CI green, sanitization done correctly), I'm closing the PR rather than merging it. The reasoning is about where this functionality lives, not whether it should exist.

What's good about the PR

• Code is clean. 25 tests pass. CI green on 3.11/3.12/3.13.
• The earlier blockers (hardcoded private IPs, force-pushed sanitization) were all addressed.
• The candidate state-machine (candidate → approved | rejected), the fail-closed approval gate, the SHA256 content hashing, and the external_memory_providers.json opt-in registration are all well thought through.
• @IkaRiche's KiLu-backed live-validation slot on Support provider-neutral external memory approval queues #1980 shows real downstream interest.

Why the WebUI repo is the wrong home

Always-visible UI for a niche workflow. The MEMORY_SECTIONS change adds an "External Memory" item to the Memory panel sidebar for every Hermes WebUI user, regardless of whether they have any external memory provider configured. Clicking it shows an empty state pointing at external_memory_providers.json. That's a permanent navigation item baked into the default chrome for a workflow that ~2 users (so far) will adopt. The WebUI's standing principle is default-off, hidden-until-opted-in for niche features (RTL toggle, quota chip, etc. — all gated). A new always-on sidebar section doesn't meet that bar.

Plugin layer is the right abstraction. Hermes Agent already has 10 memory plugins (byterover, hindsight, holographic, honcho, mem0, openviking, retaindb, supermemory, and more under hermes-agent/plugins/memory/). The WebUI's design has been to route memory through hermes_cli.memory, not to host plugin-specific UI inside WebUI itself. This PR introduces a parallel WebUI-owned approval queue that doesn't compose with the existing plugin ecosystem — if someone adds external memory to Hindsight or mem0, they'd have to wire it through both layers.

Indexing protocol coupling. Despite the provider-neutral framing on the WebUI surface, api/external_memory.py hardcodes the Ollama embed API shape and the Qdrant points upsert format. If the agent layer changes how plugins expose embed/index actions, the WebUI's copy diverges. This is the kind of tight coupling that lives well in the agent layer (which already abstracts these via plugin contracts) and lives poorly in the WebUI.

Marginal-benefit screen at 1377 LOC. The WebUI repo is conservative about feature surface area. 1377 LOC of new architecture for a feature that two contributors are excited about — without first having a design conversation that the maintainer signed off on — is exactly the wrong ratio. Compare: every recently-merged feature here is ≤200 LOC or has been broken into composable hardening passes.

Recommended path forward

The work shouldn't be wasted. The right placement is:

1. Move the approval-queue logic into a hermes-agent memory plugin at hermes-agent/plugins/memory/external_review/ (or as an extension to an existing plugin like Hindsight or mem0). The ProviderSpec, candidates table schema, approve_candidate / reject_candidate / update_candidate_text actions, and the fail-closed check_write_policy gate all translate naturally. The plugin would expose a supports_review: True capability flag and list_review_candidates() / review_candidate(...) methods.

2. Surface review in the WebUI through hermes_cli.memory as a small (~50 LOC) follow-up PR that adds:

• A "Pending memory reviews" affordance in the existing Memory panel, conditionally rendered ONLY when hermes_cli.memory.has_pending_reviews() returns truthy for the active profile
• Generic review actions (approve / reject / edit) routed through hermes_cli.memory so any plugin that exposes the contract can use them
• No new external_memory_providers.json — config goes in the plugin's existing config slot

3. Keep #1980 open as the design venue. The shape we landed on through review (three-gate model: review → write-policy → persistence with verifiable read-back) is provider-agnostic by design, so a hermes-agent plugin implementing those same fail-closed invariants is what the validation pass on #1980 should test.

What I'm doing right now

• Closing this PR and removing the maintainer-review label
• Leaving Support provider-neutral external memory approval queues #1980 OPEN as the design discussion venue
• Pinging @IkaRiche on Support provider-neutral external memory approval queues #1980 with this redirect so the KiLu validation slot is preserved against whatever lands in hermes-agent instead

If a hermes-agent PR lands implementing the plugin contract, I'm happy to fast-track the small WebUI affordance that surfaces it (~50 LOC, opt-in by plugin capability). That's the version this can ship as.

Thank you @kiosvantra for the thoughtful design and patient iteration. The contract you converged on is the right shape — it just belongs one layer down.

[kiosvantra]

Thanks for the thorough re-review and for spelling out the ownership boundary.

That makes sense to me. The fail-closed approval flow and provider-neutral contract are the parts I most wanted to validate here; if the WebUI should stay default-off / hidden-until-opted-in for this class of workflow, moving the approval queue into the hermes-agent plugin layer is the better fit.

I’ll treat #1975 as a validated prototype and carry the useful pieces forward there:

• candidate → approved/rejected state machine
• review → write-policy → persistence verification ordering
• no durable approved state before provider read-back succeeds
• provider-neutral registration/config shape
• synthetic fixtures and privacy-safe defaults

Thanks again for the review.