Skip to content

test(memory): inventory guard for the protocol-mandated update_memory_md tool - #4631

Closed
M3gA-Mind wants to merge 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/GH-4458-memory-update-md-tool-register
Closed

test(memory): inventory guard for the protocol-mandated update_memory_md tool#4631
M3gA-Mind wants to merge 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/GH-4458-memory-update-md-tool-register

Conversation

@M3gA-Mind

@M3gA-Mind M3gA-Mind commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Closes #4458

On current main, the substantive parts of #4458 are already in place: UpdateMemoryMdTool is registered in all_tools_with_runtime, its write is atomic (temp-file + rename) and serialized behind a per-workspace async lock + inter-process flock, and remember_preference/save_preference are an explicit MemoryOp::Other arm. The one remaining gap from the fix plan was the registry inventory assertion.

This adds a MEMORY_INDEX_UPDATE_TOOL constant in memory_protocol.rs (the single name classify_memory_op matches on) and a test that builds the real all_tools registry and asserts the protocol-mandated tool is registered. If the protocol ever mandates a tool that isn't in the registry, this test fails instead of the model hitting a permanent unsatisfiable "call update_memory_md" nag loop in production.

Summary by CodeRabbit

  • Bug Fixes
    • Improved memory update handling to avoid “unknown tool” failures during the memory sync flow.
    • Ensured the required memory index update action is consistently recognized when editing the memory file.
  • Tests
    • Added a regression test to confirm the full tool set includes the required memory update action.

…is registered

The memory read→dedupe→write→update-index protocol appends a "call
update_memory_md" reminder after every durable write and only that tool
can close the cycle (MemoryOp::IndexUpdate). If it is missing from the
production registry the model hits a permanent, unsatisfiable nag loop.

Registration, atomic+locked writes, and remember_preference/save_preference
classification already landed on main; the missing piece from tinyhumansai#4458 was an
inventory assertion. Name the mandated tool via a MEMORY_INDEX_UPDATE_TOOL
constant (used by classify_memory_op) and add a test that builds all_tools
and asserts the constant is registered, so a mandated tool absent from the
registry fails a test instead of production.
@M3gA-Mind
M3gA-Mind requested a review from a team July 7, 2026 13:00
@M3gA-Mind

Copy link
Copy Markdown
Collaborator Author

Closing: the #4458 fix already merged in #4495; this PR only adds a regression-guard test. Standing the fleet down on this stale-issue batch, so closing to keep the queue clean. The guard idea is noted if the maintainers want it later.

@M3gA-Mind M3gA-Mind closed this Jul 7, 2026
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7187e6a5-f42a-49aa-8f5f-f0c650fa880f

📥 Commits

Reviewing files that changed from the base of the PR and between 2ce5e60 and 723d3ff.

📒 Files selected for processing (2)
  • src/openhuman/agent/harness/memory_protocol.rs
  • src/openhuman/tools/ops_tests.rs

📝 Walkthrough

Walkthrough

A new public constant MEMORY_INDEX_UPDATE_TOOL is introduced for the mandated memory-protocol tool name, replacing a hard-coded string literal in classify_memory_op. A regression test verifies the tool is registered in the full all_tools registry.

Changes

Memory Index-Update Tool Constant

Layer / File(s) Summary
Constant definition and classification wiring
src/openhuman/agent/harness/memory_protocol.rs
Adds MEMORY_INDEX_UPDATE_TOOL constant documenting the protocol-mandated tool name, and updates classify_memory_op to match against this constant instead of the string literal.
Registry inventory regression test
src/openhuman/tools/ops_tests.rs
Adds a test building the full all_tools registry and asserting MEMORY_INDEX_UPDATE_TOOL is registered, guarding against the unknown-tool enforcement loop.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Poem

A tool once hid, unnamed, unseen,
Causing nags where writes had been.
Now MEMORY_INDEX_UPDATE_TOOL stands tall,
Registered, tested, answering the call.
No more loops — the rabbit hops free! 🐇📖


Comment @coderabbitai help to get the list of available commands.

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.

Memory protocol mandates unregistered update_memory_md tool — permanent index-drift nag loop; non-atomic MEMORY.md writes

2 participants