Skip to content

fix: hash full content in tool_add_drawer drawer ID#716

Merged
igorls merged 3 commits intoMemPalace:developfrom
shafdev:fix/drawer-id-full-content-hash
Apr 13, 2026
Merged

fix: hash full content in tool_add_drawer drawer ID#716
igorls merged 3 commits intoMemPalace:developfrom
shafdev:fix/drawer-id-full-content-hash

Conversation

@shafdev
Copy link
Copy Markdown
Contributor

@shafdev shafdev commented Apr 12, 2026

Fixes #715

What does this PR do?

tool_add_drawer generated drawer IDs by hashing only the first 100 characters
of content (content[:100]). Two documents in the same wing and room whose first
100 characters are identical received the same ID — the second upsert silently
overwrote the first with no error or warning.

Before:

drawer_id = f"drawer_{wing}_{room}_{hashlib.sha256((wing + room + content[:100]).encode()).hexdigest()[:24]}"

After:

drawer_id = f"drawer_{wing}_{room}_{hashlib.sha256((wing + room + content).encode()).hexdigest()[:24]}"

A 97-character company template header (common in Notion exports, wiki pages,
meeting notes) is enough to trigger the collision. The fix hashes the full
content, matching the uniqueness guarantee users expect from tool_add_drawer.

Note on backward compatibility: Drawers filed before this fix will not be
deduplicated against new filings of identical content — the ID scheme changed.
No existing data is deleted or corrupted.

How to test

# Run the targeted tests
python -m pytest tests/test_mcp_server.py -v

# Run the full suite
python -m pytest tests/ -v

A new regression test test_add_drawer_shared_header_no_collision in
tests/test_mcp_server.py verifies that two documents sharing a 97-character
header receive distinct drawer IDs.

Checklist

  • Tests pass (python -m pytest tests/ -v)
  • No hardcoded paths
  • Linter passes (ruff check .)

@bensig
Copy link
Copy Markdown
Collaborator

bensig commented Apr 12, 2026

@shafdev lint

@bensig
Copy link
Copy Markdown
Collaborator

bensig commented Apr 12, 2026

the lint failure is not your code — it's a pre-existing formatting issue that was fixed on develop (#675). pls merge develop into your branch or run ruff format . and push. that should clear it.

@bensig
Copy link
Copy Markdown
Collaborator

bensig commented Apr 13, 2026

The lint failure here is from a pre-existing formatting issue in convo_miner.py that was fixed in #741 (now merged). A rebase on develop should make CI pass: git fetch origin && git rebase origin/develop

@shafdev shafdev force-pushed the fix/drawer-id-full-content-hash branch from 066808a to 73c7c75 Compare April 13, 2026 03:25
@shafdev
Copy link
Copy Markdown
Contributor Author

shafdev commented Apr 13, 2026

Done — rebased on develop and fixed the ruff format mismatch (local ruff 0.15 vs CI ruff 0.4). All checks passing now.

@igorls igorls merged commit f422604 into MemPalace:develop Apr 13, 2026
6 checks passed
igorls added a commit that referenced this pull request Apr 13, 2026
PR #761 bumped pyproject.toml to 3.2.0 but missed three other version strings,
causing test_version_consistency to fail on develop CI (macos, linux 3.11, windows).

- mempalace/version.py: 3.1.0 → 3.2.0 (unblocks test_version_consistency)
- README.md: version badge shield 3.1.0 → 3.2.0
- integrations/openclaw/SKILL.md: 3.1.0 → 3.2.0
- CHANGELOG.md: rename [Unreleased] → [3.2.0] — 2026-04-13, add entries
  for #685, #690, #707, #716, #734, #755, #757, #761

Verified locally: 689/689 tests pass, ruff clean.
@igorls igorls mentioned this pull request Apr 13, 2026
4 tasks
sha2fiddy pushed a commit to sha2fiddy/mempalace that referenced this pull request Apr 13, 2026
PR MemPalace#761 bumped pyproject.toml to 3.2.0 but missed three other version strings,
causing test_version_consistency to fail on develop CI (macos, linux 3.11, windows).

- mempalace/version.py: 3.1.0 → 3.2.0 (unblocks test_version_consistency)
- README.md: version badge shield 3.1.0 → 3.2.0
- integrations/openclaw/SKILL.md: 3.1.0 → 3.2.0
- CHANGELOG.md: rename [Unreleased] → [3.2.0] — 2026-04-13, add entries
  for MemPalace#685, MemPalace#690, MemPalace#707, MemPalace#716, MemPalace#734, MemPalace#755, MemPalace#757, MemPalace#761

Verified locally: 689/689 tests pass, ruff clean.
sha2fiddy pushed a commit to sha2fiddy/mempalace that referenced this pull request Apr 14, 2026
PR MemPalace#761 bumped pyproject.toml to 3.2.0 but missed three other version strings,
causing test_version_consistency to fail on develop CI (macos, linux 3.11, windows).

- mempalace/version.py: 3.1.0 → 3.2.0 (unblocks test_version_consistency)
- README.md: version badge shield 3.1.0 → 3.2.0
- integrations/openclaw/SKILL.md: 3.1.0 → 3.2.0
- CHANGELOG.md: rename [Unreleased] → [3.2.0] — 2026-04-13, add entries
  for MemPalace#685, MemPalace#690, MemPalace#707, MemPalace#716, MemPalace#734, MemPalace#755, MemPalace#757, MemPalace#761

Verified locally: 689/689 tests pass, ruff clean.
igorls added a commit that referenced this pull request Apr 14, 2026
Main had 9 commits that never back-merged into develop after the v3.2.0
release cycle. Resolving conflicts as follows:

- mempalace/version.py: keep develop (3.3.0 release target)
- README.md: keep develop (Milla's #835 audit is authoritative — main
  had stale 19 tools / 170 tokens / "30x lossless" / v3.0.0 label)
- hooks/mempal_{save,precompact}_hook.sh: keep develop (#786 reversed
  the #666 "decision=block" behavior intentionally to stop hooks from
  making agents write in chat)
- pyproject.toml: auto-merged — keeps develop's 3.3.0 and picks up
  main's chromadb upper-bound removal (#690)
- CONTRIBUTING.md, mempalace/hooks_cli.py: auto-merged cleanly —
  picks up main's improvements (fork-first clone, more detailed
  block reason strings that name MemPalace and specific tools)
- integrations/openclaw/SKILL.md: bumped 3.2.0 → 3.3.0 (version
  tracks the package per #761 convention)
- CHANGELOG.md: manual merge — kept develop's preamble + Unreleased
  v3.3.0 section + footer links; folded main's richer v3.2.0 entries
  (Packaging section for #690/#761; Bug Fixes #685/#677/#716/#707/
  #755/#757; Documentation #734/#733) into the v3.2.0 section;
  deduped the split Documentation sections that auto-merge produced
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.

fix: tool_add_drawer silently overwrites documents with shared 100-char prefix

3 participants