From b58f85c00368cf52b0d268ec0369cc3ae527b56c Mon Sep 17 00:00:00 2001 From: "Sahil (AI)" <266772320+sahilm-ai@users.noreply.github.com> Date: Thu, 28 May 2026 15:22:58 +0530 Subject: [PATCH] feat(memory): procedural-content gate + bypass flag + tests + skill docs - memory_tool.py: _detect_procedural_content() with 4 heuristics (.md paths, SQL/code/shell, numbered steps, signal word near verb); wired into add() and replace() before size check; bypass_procedural_check=True param - tests/tools/test_memory_procedural_gate.py: 39 tests covering all 4 patterns and bypass flag - skills/autonomous-ai-agents/hermes-agent/SKILL.md: 'Memory - When NOT to use it' section with the 4 anti-patterns, bypass guidance, 3-layer-defence - scripts/release.py: add sahil.ai@ti.trilogy.com and 97122673+sahilm-ti@users.noreply.github.com to AUTHOR_MAP --- scripts/release.py | 2 + .../hermes-agent/SKILL.md | 36 ++ tests/tools/test_memory_procedural_gate.py | 321 ++++++++++++++++++ tools/memory_tool.py | 124 ++++++- 4 files changed, 478 insertions(+), 5 deletions(-) create mode 100644 tests/tools/test_memory_procedural_gate.py diff --git a/scripts/release.py b/scripts/release.py index e52f511e31c4e..e6f065f3b47f0 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -60,8 +60,10 @@ # sahilm-ti "sahil.marwaha@trilogy.com": "sahilm-ti", "sahil@nousresearch.com": "sahilm-ti", + "97122673+sahilm-ti@users.noreply.github.com": "sahilm-ti", # sahilm-ai (dedicated AI persona for agent-generated commits) "266772320+sahilm-ai@users.noreply.github.com": "sahilm-ai", + "sahil.ai@ti.trilogy.com": "sahilm-ai", # teknium (multiple emails) "teknium1@gmail.com": "teknium1", "kenyon1977@gmail.com": "kenyonxu", diff --git a/skills/autonomous-ai-agents/hermes-agent/SKILL.md b/skills/autonomous-ai-agents/hermes-agent/SKILL.md index a93c0ef0f0ec9..7fe5ad291250b 100644 --- a/skills/autonomous-ai-agents/hermes-agent/SKILL.md +++ b/skills/autonomous-ai-agents/hermes-agent/SKILL.md @@ -1019,3 +1019,39 @@ Types: `fix:`, `feat:`, `refactor:`, `docs:`, `chore:` - Use `get_hermes_home()` from `hermes_constants` for all paths (profile-safe) - Config values go in `config.yaml`, secrets go in `.env` - New tools need a `check_fn` so they only appear when requirements are met + +--- + +## Memory — When NOT to use it + +Memory is for durable **user/env facts** that survive across sessions and aren't easily re-discovered. +It is NOT a scratch pad for procedures, recipes, or skill content. + +### Anti-patterns the tool gate rejects + +The `memory` tool automatically rejects entries that match any of these: + +1. **File paths with `/references/` or ending in `.md`** — signals skill content being duplicated into memory. + Put it in a skill with `skill_manage` instead. +2. **SQL queries, shell commands, or code blocks** — these are procedures, not facts. + Use a skill's `references/` directory. +3. **Numbered steps** (`1.` / `2.` / `(1)` / `(2)` patterns) — step-by-step recipes belong in skills. +4. **Procedural signal words near imperative verbs** — phrases like "via run", "use recipe", + "procedure: run/fix/check" indicate a recipe, not a fact. + +### Bypass for legitimate edge cases + +Set `bypass_procedural_check=True` only when the entry is a genuine env fact that happens +to contain a path or short command. Examples: + +- `AWS_PROFILE=mcp-hive` (env var with a value that looks like a path) +- `Project root is ~/Desktop/work/myapp` (stable fact, not a procedure) +- `AGENTS.md governs all contributors to this repo` (reference to a file as a fact, not a recipe) + +Do NOT use the bypass to force procedures or recipes into memory. Those belong in skills. + +### The three-layer defence + +Memory bloat follows a pattern: the tool gate is the hard layer, the skill rule is the soft +layer, and the weekly watchdog (cron job `memory-audit-watchdog`) catches drift that slips +through both. If memory reaches >70% of the 2200-char cap, the watchdog pings for a manual audit. diff --git a/tests/tools/test_memory_procedural_gate.py b/tests/tools/test_memory_procedural_gate.py new file mode 100644 index 0000000000000..aec01c960e1bd --- /dev/null +++ b/tests/tools/test_memory_procedural_gate.py @@ -0,0 +1,321 @@ +"""Tests for the procedural-content gate in tools/memory_tool.py. + +Covers all four anti-patterns the gate rejects, plus the bypass flag +for legitimate env facts, and false-positive safety for common durable facts. +""" + +import json +import pytest + +from tools.memory_tool import ( + MemoryStore, + _detect_procedural_content, + memory_tool, + _PROCEDURAL_REJECTION_MSG, +) + + +# ========================================================================= +# _detect_procedural_content unit tests +# ========================================================================= + + +class TestProceduralGateHeuristic1_FilePath: + """Heuristic 1 — /references/ path or .md suffix.""" + + def test_references_path_blocked(self): + content = "Fix is in kanban-orchestrator/references/stuck-dispatch-and-missing-pings.md" + result = _detect_procedural_content(content) + assert result == _PROCEDURAL_REJECTION_MSG + + def test_any_md_file_blocked(self): + result = _detect_procedural_content("See SKILL.md for the recipe") + assert result == _PROCEDURAL_REJECTION_MSG + + def test_inline_references_path_blocked(self): + result = _detect_procedural_content( + "Recipe in kanban-orchestrator/references/human-review-approvals-and-force-push-gates.md" + ) + assert result == _PROCEDURAL_REJECTION_MSG + + def test_dotmd_extension_blocked(self): + result = _detect_procedural_content("braintrust-eng-process/references/ac-enumeration.md") + assert result == _PROCEDURAL_REJECTION_MSG + + +class TestProceduralGateHeuristic2_SqlCode: + """Heuristic 2 — SQL, code blocks, shell-command lines.""" + + def test_sql_select_blocked(self): + result = _detect_procedural_content("SELECT json_extract(payload,'$.reason') FROM task_events") + assert result == _PROCEDURAL_REJECTION_MSG + + def test_sql_update_blocked(self): + result = _detect_procedural_content( + "UPDATE tasks SET claim_lock=NULL,claim_expires=NULL WHERE id='t_abc';" + ) + assert result == _PROCEDURAL_REJECTION_MSG + + def test_sql_insert_blocked(self): + result = _detect_procedural_content( + "INSERT INTO task_events (task_id, kind) VALUES ('t_abc', 'rejected');" + ) + assert result == _PROCEDURAL_REJECTION_MSG + + def test_triple_backtick_code_block_blocked(self): + result = _detect_procedural_content("```bash\ngit push origin main\n```") + assert result == _PROCEDURAL_REJECTION_MSG + + def test_shell_command_at_line_start_blocked(self): + result = _detect_procedural_content("Fix editable install:\ncd ~/.hermes/hermes-agent") + assert result == _PROCEDURAL_REJECTION_MSG + + def test_git_command_at_line_start_blocked(self): + result = _detect_procedural_content("git push origin HEAD:my-branch") + assert result == _PROCEDURAL_REJECTION_MSG + + def test_uv_command_at_line_start_blocked(self): + result = _detect_procedural_content("uv pip install -e . --no-deps") + assert result == _PROCEDURAL_REJECTION_MSG + + def test_hermes_command_blocked(self): + result = _detect_procedural_content("hermes cron run ") + assert result == _PROCEDURAL_REJECTION_MSG + + +class TestProceduralGateHeuristic3_NumberedSteps: + """Heuristic 3 — numbered-step markers.""" + + def test_numbered_steps_blocked(self): + result = _detect_procedural_content( + "OPS HYGIENE: (1) verify artifacts. (2) kill -9 workers. (3) fix editable install." + ) + assert result == _PROCEDURAL_REJECTION_MSG + + def test_dot_numbered_steps_blocked(self): + result = _detect_procedural_content("1. clone repo\n2. install deps\n3. run tests") + assert result == _PROCEDURAL_REJECTION_MSG + + def test_single_numbered_step_blocked(self): + # Even a single "1. do something" is a recipe indicator + result = _detect_procedural_content("1. run `git fetch` to pick up upstream changes") + assert result == _PROCEDURAL_REJECTION_MSG + + +class TestProceduralGateHeuristic4_SignalNearVerb: + """Heuristic 4 — procedural signal word near imperative verb.""" + + def test_via_plus_verb_blocked(self): + # "via terminal" near an imperative verb + result = _detect_procedural_content( + "APPROVAL→MERGE: merge it via kanban_approve flow" + ) + assert result == _PROCEDURAL_REJECTION_MSG + + def test_recipe_blocked(self): + result = _detect_procedural_content("Full recipe: run the audit first then verify") + assert result == _PROCEDURAL_REJECTION_MSG + + def test_procedure_blocked(self): + result = _detect_procedural_content( + "Standard procedure is to run the check and verify output" + ) + assert result == _PROCEDURAL_REJECTION_MSG + + def test_flow_colon_blocked(self): + result = _detect_procedural_content( + "flow: kanban_show → find PR → gh pr view → merge if clean" + ) + assert result == _PROCEDURAL_REJECTION_MSG + + +# ========================================================================= +# Bypass flag — legitimate env facts that trip the heuristics +# ========================================================================= + + +class TestProceduralGateBypass: + """bypass_procedural_check=True lets env facts with path patterns through.""" + + def test_bypass_env_fact_with_path(self): + # A stable env fact that happens to contain a file path + result = _detect_procedural_content( + "AWS_PROFILE=mcp-hive points at ~/.aws/credentials" + ) + # This does NOT contain .md or /references/ — should not trigger + # (Testing that bypass isn't needed for this particular fact) + # But we test the bypass mechanism via MemoryStore below + + @pytest.fixture() + def store(self, tmp_path, monkeypatch): + monkeypatch.setattr("tools.memory_tool.get_memory_dir", lambda: tmp_path) + s = MemoryStore(memory_char_limit=500, user_char_limit=300) + s.load_from_disk() + return s + + def test_store_add_blocks_md_path_by_default(self, store): + result = store.add("memory", "See SKILL.md for the recipe") + assert result["success"] is False + assert "procedure" in result["error"].lower() + + def test_store_add_bypasses_with_flag(self, store): + # A genuine env fact that contains an .md path the gate would block + content = "Project conventions in AGENTS.md govern all contributors" + result = store.add("memory", content, bypass_procedural_check=True) + assert result["success"] is True + + def test_store_replace_blocks_procedural_by_default(self, store): + store.add("memory", "initial fact", bypass_procedural_check=True) + result = store.replace("memory", "initial fact", "1. do this 2. do that") + assert result["success"] is False + assert "procedure" in result["error"].lower() + + def test_store_replace_bypasses_with_flag(self, store): + store.add("memory", "initial fact", bypass_procedural_check=True) + result = store.replace( + "memory", + "initial fact", + "Project root is ~/Desktop/work/myapp/AGENTS.md area", + bypass_procedural_check=True, + ) + assert result["success"] is True + + +# ========================================================================= +# memory_tool() dispatcher integration +# ========================================================================= + + +class TestMemoryToolDispatcherProcedural: + """End-to-end: memory_tool() -> MemoryStore -> procedural gate.""" + + @pytest.fixture() + def store(self, tmp_path, monkeypatch): + monkeypatch.setattr("tools.memory_tool.get_memory_dir", lambda: tmp_path) + s = MemoryStore(memory_char_limit=500, user_char_limit=300) + s.load_from_disk() + return s + + def test_add_numbered_steps_rejected(self, store): + result = json.loads( + memory_tool( + action="add", + target="memory", + content="(1) verify artifacts (2) clear stale lock (3) reinstall editable", + store=store, + ) + ) + assert result["success"] is False + assert "procedure" in result["error"].lower() + + def test_add_references_path_rejected(self, store): + result = json.loads( + memory_tool( + action="add", + target="memory", + content="Full recipe in kanban-orchestrator/references/stuck-dispatch.md", + store=store, + ) + ) + assert result["success"] is False + assert "procedure" in result["error"].lower() + + def test_add_sql_rejected(self, store): + result = json.loads( + memory_tool( + action="add", + target="memory", + content="SELECT * FROM tasks WHERE status='ready'", + store=store, + ) + ) + assert result["success"] is False + assert "procedure" in result["error"].lower() + + def test_add_code_block_rejected(self, store): + result = json.loads( + memory_tool( + action="add", + target="memory", + content="Fix with: ```bash\ncd repo && pip install -e .\n```", + store=store, + ) + ) + assert result["success"] is False + assert "procedure" in result["error"].lower() + + def test_add_bypass_flag_works(self, store): + """Legitimate env fact with path goes through with bypass_procedural_check=True.""" + result = json.loads( + memory_tool( + action="add", + target="memory", + content="Project conventions in AGENTS.md govern all contributors", + store=store, + bypass_procedural_check=True, + ) + ) + assert result["success"] is True + + def test_add_clean_fact_passes(self, store): + """Durable env facts without any procedural signals pass without bypass.""" + result = json.loads( + memory_tool( + action="add", + target="memory", + content="User prefers dark mode and concise responses", + store=store, + ) + ) + assert result["success"] is True + + def test_add_env_var_fact_passes(self, store): + """Env-var style facts pass without bypass.""" + result = json.loads( + memory_tool( + action="add", + target="memory", + content="AWS_PROFILE=mcp-hive is the default AWS profile for BrainTrust work", + store=store, + ) + ) + assert result["success"] is True + + +# ========================================================================= +# False-positive safety — common durable facts should NOT be blocked +# ========================================================================= + + +class TestProceduralGateFalsePositives: + """Common durable user/env facts that the gate must not block.""" + + def test_user_preference_passes(self): + assert _detect_procedural_content("User prefers dark mode") is None + + def test_env_fact_passes(self): + assert _detect_procedural_content("Project uses Python 3.12 with FastAPI") is None + + def test_provider_fact_passes(self): + assert _detect_procedural_content("Main LLM provider is Anthropic, model claude-sonnet-4") is None + + def test_team_fact_passes(self): + assert _detect_procedural_content("Sahil runs the braintrust team at Trilogy Innovations") is None + + def test_aws_profile_without_path_passes(self): + assert _detect_procedural_content("AWS_PROFILE=mcp-hive is the default BrainTrust AWS profile") is None + + def test_git_identity_fact_passes(self): + # Doesn't start with a shell command verb at line start + assert _detect_procedural_content("Git identity: sahilm-ai, OAuth token in GH_TOKEN_SAHILM_AI") is None + + def test_tool_quirk_fact_passes(self): + assert _detect_procedural_content("Telegram does not render pipe tables") is None + + def test_synapse_os_ids_pass(self): + assert ( + _detect_procedural_content( + "SYNAPSE OS: team team.trilogy-innovations, project project.braintrust, OKR f1320919" + ) + is None + ) diff --git a/tools/memory_tool.py b/tools/memory_tool.py index 5b9af55928e4d..6462f76fdf1ce 100644 --- a/tools/memory_tool.py +++ b/tools/memory_tool.py @@ -81,6 +81,92 @@ def _scan_memory_content(content: str) -> Optional[str]: return _first_threat_message(content, scope="strict") +# --------------------------------------------------------------------------- +# Procedural-content gate — reject recipes / procedures / skill duplicates +# at write time so memory stays bounded to durable facts. +# --------------------------------------------------------------------------- + +_PROCEDURAL_REJECTION_MSG = ( + "Memory rejected: this looks like a procedure (numbered steps / file path / SQL / command). " + "Memory is for durable user/env facts. Put procedures in a skill: skill_view or skill_manage. " + "If this is genuinely a user-preference and the heuristic is wrong, set bypass_procedural_check=True." +) + +# Imperative verbs frequently found in recipe prose. Order does not matter — +# all are checked. We only trigger when a DIFFERENT procedural signal word +# appears within ±50 chars of one of these (no self-match). +_IMPERATIVE_VERBS: List[str] = [ + "run", "call", "set", "add", "remove", "check", "apply", "create", + "install", "execute", "do", "merge", "push", "pull", "patch", + "read", "write", "verify", "fetch", "clear", "fix", "restart", + "update", "open", "close", "enable", "disable", "delete", "insert", +] + +# Words whose presence is a strong signal that the entry is procedural. +_PROCEDURAL_SIGNALS: List[str] = [ + "via", "use", "run", "flow:", "recipe", "procedure", +] + + +def _detect_procedural_content(content: str) -> Optional[str]: + """Return a rejection string if *content* looks procedural, else None. + + Four heuristics (any one triggers rejection): + + 1. A file path referencing ``/references/`` or ending in ``.md`` — + indicates skill content being duplicated into memory. + 2. An SQL keyword, triple-backtick code block, or shell-command prompt. + 3. Numbered-step markers like ``1.`` / ``2.`` or ``(1)`` / ``(2)``. + 4. A procedural signal word (``via``, ``use``, ``run``, ``recipe``, + ``procedure``, ``flow:``) within ±50 chars of an imperative verb + (excluding self-match so e.g. ``use`` alone does not trigger). + + To bypass for a legitimate edge case, pass ``bypass_procedural_check=True`` + to the memory tool. + """ + # --- Heuristic 1: skill/doc file path signals --- + if "/references/" in content: + return _PROCEDURAL_REJECTION_MSG + if re.search(r"\S+\.md\b", content): + return _PROCEDURAL_REJECTION_MSG + + # --- Heuristic 2: SQL, code blocks, shell commands --- + if re.search(r"\b(SELECT|INSERT|UPDATE|DELETE|CREATE\s+TABLE|ALTER\s+TABLE)\b", content, re.IGNORECASE): + return _PROCEDURAL_REJECTION_MSG + if "```" in content: + return _PROCEDURAL_REJECTION_MSG + # Shell-command prompt ($ or # at line start) or common shell verbs at line start + if re.search(r"(?m)^[$#] .+", content): + return _PROCEDURAL_REJECTION_MSG + if re.search( + r"(?m)^(cd|git|pip|uv|npm|yarn|python|hermes|gh|curl|sed|awk|grep|find|brew)\s", + content, + ): + return _PROCEDURAL_REJECTION_MSG + + # --- Heuristic 3: numbered steps --- + if re.search(r"\(\d+\)\s+\S|\b\d+\.\s+\S", content): + return _PROCEDURAL_REJECTION_MSG + + # --- Heuristic 4: procedural signal word near an imperative verb --- + lower = content.lower() + for signal in _PROCEDURAL_SIGNALS: + pos = lower.find(signal) + while pos != -1: + start = max(0, pos - 50) + end = min(len(lower), pos + len(signal) + 50) + window = lower[start:end] + for verb in _IMPERATIVE_VERBS: + if verb == signal: + # Skip self-match (e.g. "run" signal near "run" verb) + continue + if re.search(r"\b" + re.escape(verb) + r"\b", window): + return _PROCEDURAL_REJECTION_MSG + pos = lower.find(signal, pos + 1) + + return None + + def _drift_error(path: "Path", bak_path: str) -> Dict[str, Any]: """Build the error dict returned when external drift is detected. @@ -295,7 +381,7 @@ def _char_limit(self, target: str) -> int: return self.user_char_limit return self.memory_char_limit - def add(self, target: str, content: str) -> Dict[str, Any]: + def add(self, target: str, content: str, bypass_procedural_check: bool = False) -> Dict[str, Any]: """Append a new entry. Returns error if it would exceed the char limit.""" content = content.strip() if not content: @@ -306,6 +392,12 @@ def add(self, target: str, content: str) -> Dict[str, Any]: if scan_error: return {"success": False, "error": scan_error} + # Procedural-content gate — reject recipes/procedures unless bypassed + if not bypass_procedural_check: + proc_error = _detect_procedural_content(content) + if proc_error: + return {"success": False, "error": proc_error} + with self._file_lock(self._path_for(target)): # Re-read from disk under lock to pick up writes from other sessions. # If external drift was detected, the file was backed up to .bak. @@ -345,7 +437,7 @@ def add(self, target: str, content: str) -> Dict[str, Any]: return self._success_response(target, "Entry added.") - def replace(self, target: str, old_text: str, new_content: str) -> Dict[str, Any]: + def replace(self, target: str, old_text: str, new_content: str, bypass_procedural_check: bool = False) -> Dict[str, Any]: """Find entry containing old_text substring, replace it with new_content.""" old_text = old_text.strip() new_content = new_content.strip() @@ -359,6 +451,12 @@ def replace(self, target: str, old_text: str, new_content: str) -> Dict[str, Any if scan_error: return {"success": False, "error": scan_error} + # Procedural-content gate — reject recipes/procedures unless bypassed + if not bypass_procedural_check: + proc_error = _detect_procedural_content(new_content) + if proc_error: + return {"success": False, "error": proc_error} + with self._file_lock(self._path_for(target)): bak = self._reload_target(target) if bak: @@ -606,6 +704,7 @@ def memory_tool( content: str = None, old_text: str = None, store: Optional[MemoryStore] = None, + bypass_procedural_check: bool = False, ) -> str: """ Single entry point for the memory tool. Dispatches to MemoryStore methods. @@ -621,14 +720,14 @@ def memory_tool( if action == "add": if not content: return tool_error("Content is required for 'add' action.", success=False) - result = store.add(target, content) + result = store.add(target, content, bypass_procedural_check=bypass_procedural_check) elif action == "replace": if not old_text: return tool_error("old_text is required for 'replace' action.", success=False) if not content: return tool_error("content is required for 'replace' action.", success=False) - result = store.replace(target, old_text, content) + result = store.replace(target, old_text, content, bypass_procedural_check=bypass_procedural_check) elif action == "remove": if not old_text: @@ -673,7 +772,12 @@ def check_memory_requirements() -> bool: "- 'memory': your notes -- environment facts, project conventions, tool quirks, lessons learned\n\n" "ACTIONS: add (new entry), replace (update existing -- old_text identifies it), " "remove (delete -- old_text identifies it).\n\n" - "SKIP: trivial/obvious info, things easily re-discovered, raw data dumps, and temporary task state." + "SKIP: trivial/obvious info, things easily re-discovered, raw data dumps, and temporary task state.\n\n" + "PROCEDURAL GATE: Entries with numbered steps, file paths ending in .md, /references/ paths, " + "SQL queries, code blocks, or shell commands are rejected automatically. " + "These belong in skills (skill_manage), not memory. " + "Set bypass_procedural_check=True only for legitimate env facts that happen to contain a path " + "(e.g. 'AWS_PROFILE=mcp-hive points at ~/.aws/credentials')." ), "parameters": { "type": "object", @@ -696,6 +800,15 @@ def check_memory_requirements() -> bool: "type": "string", "description": "Short unique substring identifying the entry to replace or remove." }, + "bypass_procedural_check": { + "type": "boolean", + "description": ( + "Set True only when the entry is a genuine user/env fact that happens to contain " + "a file path, short command, or other pattern that triggered the procedural gate. " + "Example: 'AWS_PROFILE=mcp-hive' or 'Project root is at ~/Desktop/work/myapp'. " + "Do NOT set True to force procedures/recipes into memory — put those in a skill." + ), + }, }, "required": ["action", "target"], }, @@ -714,6 +827,7 @@ def check_memory_requirements() -> bool: target=args.get("target", "memory"), content=args.get("content"), old_text=args.get("old_text"), + bypass_procedural_check=bool(args.get("bypass_procedural_check", False)), store=kw.get("store")), check_fn=check_memory_requirements, emoji="🧠",