Skip to content

fix(memory): guard stale recall and unsafe learning - #18372

Open
flyingdoubleG wants to merge 1 commit into
NousResearch:mainfrom
flyingdoubleG:codex/fix-memory-hardcase-guards
Open

fix(memory): guard stale recall and unsafe learning#18372
flyingdoubleG wants to merge 1 commit into
NousResearch:mainfrom
flyingdoubleG:codex/fix-memory-hardcase-guards

Conversation

@flyingdoubleG

@flyingdoubleG flyingdoubleG commented May 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add optional Hindsight sync_recall so auto-recall can use the current turn's query instead of consuming stale background results
  • reject agent-created skill writes that encode broad tool/environment failures as permanent avoidance rules unless scoped as transient or revalidated
  • inject a read-only project status baseline for status/continue-work turns so current git/disk evidence outranks stale memory/session summaries
  • strengthen execution guidance so memory/todo/skill housekeeping does not displace clear non-destructive user requests

Related issues

Real agent validation

Ran a real Hermes/Codex agent hardcase with an isolated HERMES_HOME:

  • stale memories/MEMORY.md: “Phase A provider/model persistence is NOT started”
  • current temp git repo STATUS.md: “Phase A provider/model persistence is COMPLETE” and “Phase B integration tests are the next remaining work”
  • command used openai-codex / gpt-5.5 with terminal,file,memory toolsets
  • result: the agent answered that Phase A is complete, explicitly preferred current git/disk baseline over stale memory, and identified Phase B integration tests as next; no files were modified

Validation

  • D:\工作\hermes-agent\.venv-win\Scripts\python.exe -m pytest tests\plugins\memory\test_hindsight_provider.py::TestPrefetch -q
  • D:\工作\hermes-agent\.venv-win\Scripts\python.exe -m pytest tests\plugins\memory\test_hindsight_provider.py::TestPrefetch::test_sync_recall_prefetch_uses_current_query tests\plugins\memory\test_hindsight_provider.py::TestPrefetch::test_sync_recall_prefetch_respects_reflect_method tests\plugins\memory\test_hindsight_provider.py::TestConfig::test_custom_config_values tests\plugins\memory\test_hindsight_provider.py::TestConfigSchema::test_schema_has_all_new_fields -q
  • D:\工作\hermes-agent\.venv-win\Scripts\python.exe -m pytest tests\tools\test_skill_manager_tool.py -q
  • D:\工作\hermes-agent\.venv-win\Scripts\python.exe -m pytest tests\run_agent\test_project_status_baseline.py tests\agent\test_prompt_execution_guards.py -q

Note: the full tests\plugins\memory\test_hindsight_provider.py file still has 3 pre-existing Windows HOME/profile-env failures in local verification; the changed prefetch/config paths pass.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/skills Skills system (list, view, manage) tool/memory Memory tool and memory providers labels May 1, 2026
@flyingdoubleG

Copy link
Copy Markdown
Contributor Author

Real-agent validation (2026-05-01): PASS

Isolated HERMES_HOME and temp git repo baseline were used for a long-context/stale-memory hardcase.
Scenario: stale long-term memory claimed Phase A provider/model persistence is NOT started, while the current repo disk/git baseline STATUS.md said Phase A provider/model persistence is COMPLETE and Phase B integration tests are next.
Agent result: the real OpenAI Codex-backed Hermes agent answered from current repo evidence, not stale memory: Phase A 已完成... 以当前仓库证据为准。下一步:Phase B integration tests。

This validates the stale-memory guard through the actual agent path.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying the stale Hindsight recall path; that premise is still present on current main (plugins/memory/hindsight/__init__.py:1466-1527, with post-turn warming at run_agent.py:3404-3412).

Problems

  • run_agent.py:934-964 directly reads host cwd/git/status files. This bypasses the session-aware resolver in agent/runtime_cwd.py:39-50 and conflicts with the remote-backend contract in agent/prompt_builder.py:1123-1128; the resulting host data is injected into the model request at PR run_agent.py:10937-10942.
  • With sync_recall, prefetch() recalls synchronously (plugins/memory/hindsight/__init__.py:1145-1198) but queue_prefetch() still starts a background recall (:1222-1246). Current synchronous Mem0 coverage requires queueing to be a no-op (tests/plugins/memory/test_mem0_v3.py:344-350).
  • The skill validator accepts a permanent avoidance rule whenever any qualifier appears elsewhere in the whole skill (tools/skill_manager_tool.py:299-305).

Suggested changes

  • Split the Hindsight fix, make sync mode suppress post-turn warming, and add its no-extra-search regression test plus README configuration documentation.
  • Rework project-status discovery through the session/backend-aware execution path rather than host-side subprocess and file reads.
  • Scope any transient exception to the specific skill instruction it qualifies.

Automated hermes-sweeper review.

Comment thread run_agent.py
if not _looks_like_project_status_request(user_message):
return ""

cwd_path = Path(cwd or os.getenv("TERMINAL_CWD") or os.getcwd()).expanduser().resolve()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not inspect cwd/git/status documents directly from run_agent. agent/runtime_cwd.py:39-50 makes the per-session cwd override canonical, while agent/prompt_builder.py:1123-1128 guarantees remote-backend tools operate in the backend rather than on the Hermes host. This bypasses both contracts and the resulting host data is injected into the model request.

logger.debug("Prefetch: recall returned %d results", num_results)
return "\n".join(f"- {r.text}" for r in resp.results if r.text) if resp.results else ""

def queue_prefetch(self, query: str, *, session_id: str = "") -> None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When sync_recall is enabled, this still launches a post-turn background recall even though prefetch() performs a current-query synchronous recall. Make this a no-op in sync mode and test that no backend recall is issued, matching the synchronous-provider invariant in tests/plugins/memory/test_mem0_v3.py:344-350.

lowered = content.lower()
if not _NEGATIVE_TOOL_LEARNING_RE.search(content):
return None
if any(token in lowered for token in _NEGATIVE_LEARNING_QUALIFIERS):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This searches the entire skill for a qualifier, so an unrelated retry or revalidate sentence permits a permanent avoidance rule elsewhere. Scope the exception to the actual matched instruction, or use structured validation rather than a document-global substring allowlist.



def test_tool_use_guidance_prioritizes_main_request_over_housekeeping():
assert "execute the main request before side-channel housekeeping" in TOOL_USE_ENFORCEMENT_GUIDANCE

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These exact-substring assertions are prompt snapshots, not a behavioral contract. Replace them with coverage of the intended runtime behavior, or omit them; AGENTS.md explicitly rejects change-detector tests.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit area/memory Memory subsystem: store, providers, sync, background reviews labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/memory Memory subsystem: store, providers, sync, background reviews comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/memory Memory tool and memory providers tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants