From dc8b790cf88d5dc530f9b3d6bc0e319b43a9b614 Mon Sep 17 00:00:00 2001 From: Sadovoi Grigorii Date: Fri, 3 Jul 2026 05:54:44 +0300 Subject: [PATCH] fix: avoid note ID collisions (#630) --- strix/tools/notes/tools.py | 17 +++++++++- tests/test_notes.py | 67 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 tests/test_notes.py diff --git a/strix/tools/notes/tools.py b/strix/tools/notes/tools.py index e9f2cdf..da6eb72 100644 --- a/strix/tools/notes/tools.py +++ b/strix/tools/notes/tools.py @@ -22,10 +22,19 @@ _notes_storage: dict[str, dict[str, Any]] = {} _VALID_NOTE_CATEGORIES = ["general", "findings", "methodology", "questions", "plan", "wiki"] _notes_lock = threading.RLock() _DEFAULT_CONTENT_PREVIEW_CHARS = 280 +_NOTE_ID_GENERATION_ATTEMPTS = 1024 _notes_path: Path | None = None +def _generate_note_id() -> str | None: + for _ in range(_NOTE_ID_GENERATION_ATTEMPTS): + note_id = uuid.uuid4().hex[:6] + if note_id not in _notes_storage: + return note_id + return None + + def hydrate_notes_from_disk(state_dir: Path) -> None: global _notes_path # noqa: PLW0603 _notes_path = state_dir / "notes.json" @@ -153,7 +162,13 @@ def _create_note_impl( "note_id": None, } - note_id = str(uuid.uuid4())[:6] + note_id = _generate_note_id() + if note_id is None: + return { + "success": False, + "error": "Failed to generate a unique note ID", + "note_id": None, + } timestamp = datetime.now(UTC).isoformat() note = { diff --git a/tests/test_notes.py b/tests/test_notes.py new file mode 100644 index 0000000..e6a63df --- /dev/null +++ b/tests/test_notes.py @@ -0,0 +1,67 @@ +"""Tests for per-run notes storage.""" + +from __future__ import annotations + +import uuid +from typing import TYPE_CHECKING + +import pytest + +import strix.tools.notes.tools as notes_tools + + +if TYPE_CHECKING: + from collections.abc import Iterator + + +@pytest.fixture(autouse=True) +def _reset_notes_storage(monkeypatch: pytest.MonkeyPatch) -> Iterator[None]: + monkeypatch.setattr(notes_tools, "_notes_path", None) + with notes_tools._notes_lock: + notes_tools._notes_storage.clear() + yield + with notes_tools._notes_lock: + notes_tools._notes_storage.clear() + + +def test_create_note_retries_on_note_id_collision(monkeypatch: pytest.MonkeyPatch) -> None: + generated_ids = iter( + [ + uuid.UUID("abcdef00-0000-4000-8000-000000000000"), + uuid.UUID("abcdef11-0000-4000-8000-000000000000"), + uuid.UUID("12345600-0000-4000-8000-000000000000"), + ] + ) + monkeypatch.setattr(notes_tools.uuid, "uuid4", lambda: next(generated_ids)) + + first = notes_tools._create_note_impl("first", "original content") + second = notes_tools._create_note_impl("second", "new content") + + assert first["success"] is True + assert first["note_id"] == "abcdef" + assert second["success"] is True + assert second["note_id"] == "123456" + assert second["total_count"] == 2 + assert notes_tools._notes_storage["abcdef"]["content"] == "original content" + assert notes_tools._notes_storage["123456"]["content"] == "new content" + + +def test_create_note_returns_error_after_repeated_note_id_collisions( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(notes_tools, "_NOTE_ID_GENERATION_ATTEMPTS", 2) + monkeypatch.setattr( + notes_tools.uuid, + "uuid4", + lambda: uuid.UUID("abcdef00-0000-4000-8000-000000000000"), + ) + notes_tools._notes_storage["abcdef"] = {"content": "existing"} + + result = notes_tools._create_note_impl("second", "new content") + + assert result == { + "success": False, + "error": "Failed to generate a unique note ID", + "note_id": None, + } + assert notes_tools._notes_storage == {"abcdef": {"content": "existing"}}