Skip to content

Tiered context memory (company inlined / team read-first / project index) - #14

Merged
GurneeshBudhiraja merged 2 commits into
mainfrom
dev/gurneesh/consult-memory-hardening
Aug 4, 2026
Merged

Tiered context memory (company inlined / team read-first / project index)#14
GurneeshBudhiraja merged 2 commits into
mainfrom
dev/gurneesh/consult-memory-hardening

Conversation

@GurneeshBudhiraja

Copy link
Copy Markdown

What

Tiered context-memory delivery in tools/memory_read_tool.py, backing the dynamic-memory-fetch work in hermes (parent PR: KNOWIDEA-Tech/hermes#122).

  • Company notes inlined in full every turn (deterministic baseline).
  • Team notes read-first (mandatory read_team_context).
  • Project notes as an index + open-relevant-by-id.
  • Adds list_scope_notes / read_notes_by_ids — the metadata index + full-body reads used by the pre-question memory selection (interpreter) and the new plan verifier on the hermes side.

Pairs with

Parent hermes memory-debug pins this submodule at 1eba4d29 (PR NousResearch#122 → staging).

Testing

Memory scope/tool tests pass (41). Tenant isolation preserved (allow-list + memory_context contextvar; service-role app-layer filters remain the boundary).

…ds), model-driven note selection, index+rules in system prompt

- New tools/memory_read_tool.py: build_context_memory_index_section (index only, no bodies),
  read_company_context/read_team_context/read_project_note, list_scope_notes, read_notes_by_ids.
  Retrieval rules: open relevant notes by note_id across all levels; company/team prioritized; contradiction -> clarify.
- model_tools.py: register tools.memory_read_tool in discovery.
- toolsets.py: memory_consult exposes read_company_context/read_team_context/read_project_note.
…irst, project index)

Company notes inlined in full every turn; team notes read-first; project
notes index + open-relevant-by-id. Adds list_scope_notes / read_notes_by_ids
for the pre-question memory selection (interpreter + plan verifier).
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

⚠️ Supply Chain Risk Detected

This PR contains patterns commonly associated with supply chain attacks. This does not mean the PR is malicious — but these patterns require careful human review before merging.

⚠️ WARNING: exec() or eval() usage

Dynamic code execution can hide malicious behavior, especially when combined with base64 or network fetches.

Matches (first 20):

2159:+        "description": "Hierarchical memory retrieval (Company/Team/Project notes) — available only on memory-context turns. read_company_context/read_team_context return FULL company/team bodies; read_project_note returns a FULL project note by id (guided by the Project Notes Index in the prompt). No truncation.",

Automated scan triggered by supply-chain-audit. If this is a false positive, a maintainer can approve after manual review.

@GurneeshBudhiraja

Copy link
Copy Markdown
Author

Review — tiered context-memory read tools

This is the enforcement layer the parent hermes#122 scoping guarantee rests on (pinned at this exact commit 1eba4d29). Read it in full — it's solid, and I'm comfortable signing off.

✅ Merge-readiness

  • CI green (test ✅, supply-chain scan ✅), mergeState CLEAN.
  • Reviewed the whole memory_read_tool.py (730 lines) + the toolsets.py/model_tools.py wiring.

Security — the important part, and it holds

The two-boundary model is correctly implemented and is the reason a model-selected id can't leak across tenants:

  • _fetch_notes re-scopes every read to client_id = org_id, where org_id = _resolve_user_org(sb, user_id) is derived server-side from the trusted user_id contextvar — never from the model — plus a personal-overlay guard (is_user_context.eq.false OR user_id.eq.<user>). A forged allow-list can't cross orgs or surface another user's private note.
  • read_project_note / read_notes_by_ids intersect the requested id(s) with allowed_note_ids before fetching; out-of-scope ids are rejected/dropped with a log.
  • Prompt-injection preamble (_MEMORY_SAFETY_PREAMBLE) is strong and matters here: shared company/team notes are authored by others in the org, and the block explicitly frames them as reference data, "never instructions," don't-follow-action-language, don't-reveal-system-prompt. Good posture.

Design notes (all minor / non-blocking)

  1. PostgREST filter is string-interpolated: .or_(f"is_user_context.eq.false,user_id.eq.{user_id}"). Safe today because user_id is a server-trusted UUID from the auth contextvar (not model input), but interpolating into a filter string is a latent footgun if that invariant ever weakens or a user_id ever contained PostgREST metacharacters. Worth a one-line comment asserting "user_id is a validated UUID" (it's inherited from memory_consult_tool, so the convention predates this PR).

  2. Node-kind resolution failure silently demotes company → project. In build_context_memory_index_section, if _resolve_node_kinds returns {} (a transient context_nodes query hiccup), company_nodes/team_nodes are empty, so company notes are neither inlined nor read-first — they land in the Project index instead. Not lost (still in the allow-list, still openable by id), but the "company is the always-applied authoritative baseline" guarantee quietly weakens to "company is somewhere in the project list" on a bad turn. Consider an explicit warn (or a soft-fail return) when scope_nodes is non-empty but kinds came back empty, so it's visible rather than silent.

  3. Redundant context_nodes lookups per turn. _resolve_node_kinds is re-queried by _read_level, build_context_memory_index_section, and list_scope_notes (via _classify_notes) independently. Small queries, but several fire on one memory turn — a per-scope cache would trim round-trips.

  4. Doc nit: read_notes_by_ids docstring advertises level_hint? in the return shape, but the code returns only {note_id, title, content} (the hermes-side memory_select tags level itself). Drop the level_hint? mention.

  5. Belt-and-suspenders surface: company bodies are inlined in the prompt AND re-readable via read_company_context/read_project_note. Harmless (the schema tells the agent it's already inlined), just noting the redundancy.

Overall: LGTM — good to merge. Findings above are polish; #2 (silent company demotion on a kinds-lookup failure) is the only one I'd consider worth a follow-up, and it's a degraded-path edge, not a normal-path bug.

@GurneeshBudhiraja
GurneeshBudhiraja merged commit 8ce6d92 into main Aug 4, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant