Merge branch 'pr2/source-at-record-time' into pr3/spool-delivery

integrations/claude-code-plugin/core/telemetry.py

@@ -49,6 +49,7 @@ except ImportError:
     _PLATFORM_SOURCE = "MEM0_PLUGIN"
     _PLATFORM_APPLICATION = ""
 
+_salt_cache: str = ""
 _harness: str = _DEFAULT_HARNESS
 _source_tag: str = _DEFAULT_SOURCE_TAG
 _PRIVATE_KEYS = {
@@ -121,15 +122,60 @@ def _digest(value: str, length: int = 16) -> str:
     return hashlib.sha256(value.encode("utf-8")).hexdigest()[:length]
 
 
+def _salt_path() -> Path:
+    return memory_core.data_dir() / "telemetry-salt"
+
+
 def _install_salt() -> str:
-    """Random per-install salt, created on first use and kept in the identity file."""
-    identity = _read_identity()
-    salt = identity.get("salt")
-    if not salt:
-        salt = uuid.uuid4().hex
-        identity["salt"] = salt
-        _write_identity(identity)
-    return salt
+    """Random per-install salt, created once and memoized for the process.
+
+    Deliberately its own file, claimed with O_CREAT|O_EXCL, rather than a key in
+    the identity file. Three reasons, all of which produced wrong data when this
+    lived in the identity dict:
+
+    - Hooks are short-lived separate processes firing on every tool call, and
+      people run more than one agent window. A read-modify-write would let each
+      process mint its own salt, so one repository would hash several ways in the
+      window before a writer won.
+    - resolve_distinct_id holds a copy of the identity dict across a network call
+      to /v1/ping/, so whichever write landed second erased the other's key —
+      losing either the salt (repo_hash changes mid-stream) or the email (a
+      second $identify, splitting the person).
+    - Touching the identity file from record() would create it, and is_first_run
+      keys off that file, so recording an event would silently suppress the
+      install event.
+
+    On a read-only or full data directory the fallback is derived from the data
+    directory path: stable for the machine rather than random per call, so the
+    failure mode is a weaker salt and not unbounded cardinality in PostHog.
+    """
+    global _salt_cache
+    if _salt_cache:
+        return _salt_cache
+
+    path = _salt_path()
+    try:
+        path.parent.mkdir(parents=True, exist_ok=True)
+        handle = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600)
+        try:
+            with os.fdopen(handle, "w", encoding="utf-8") as stream:
+                stream.write(uuid.uuid4().hex)
+        except OSError:
+            pass
+    except FileExistsError:
+        pass
+    except OSError:
+        # Cannot persist. Stable-per-machine beats random-per-call.
+        _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest()
+        return _salt_cache
+
+    try:
+        _salt_cache = path.read_text(encoding="utf-8").strip()
+    except OSError:
+        _salt_cache = ""
+    if not _salt_cache:
+        _salt_cache = hashlib.sha256(str(path).encode("utf-8")).hexdigest()
+    return _salt_cache
 
 
 def _scoped_digest(value: str, length: int = 16) -> str:

integrations/claude-code-plugin/tests/test_telemetry.py

@@ -280,3 +280,49 @@ def test_spawn_flush_does_nothing_without_a_spool(isolated_env):
     with patch.object(telemetry.subprocess, "Popen") as popen:
         assert telemetry.spawn_flush() is True
         popen.assert_called_once()
+
+
+def test_salt_is_stable_across_processes(isolated_env):
+    """Hooks are separate short-lived processes; one repo must hash one way.
+
+    An unlocked read-modify-write let each process mint its own salt, so a
+    repository hashed several ways in the window before one writer won.
+    """
+    import subprocess as sp
+
+    core = str(Path(__file__).resolve().parents[1] / "core")
+    script = (
+        f"import sys; sys.path.insert(0, {core!r})\n"
+        "import telemetry\n"
+        "print(telemetry._install_salt())"
+    )
+    env = {**os.environ, "MEM0_CODE_DATA_DIR": str(memory_core.data_dir())}
+    salts = {
+        sp.run([sys.executable, "-c", script], capture_output=True, text=True, env=env).stdout.strip()
+        for _ in range(4)
+    }
+    assert len(salts) == 1, f"one repo hashed {len(salts)} ways: {salts}"
+
+
+def test_salt_does_not_touch_the_identity_file(isolated_env):
+    """The identity file is is_first_run's marker and the sender's email store.
+
+    Writing the salt into it would create it from record(), suppressing the
+    install event, and would race resolve_distinct_id, which holds a stale copy
+    of that dict across a network call.
+    """
+    telemetry._install_salt()
+    assert not telemetry._identity_path().exists()
+
+
+def test_salt_is_stable_when_it_cannot_be_persisted(isolated_env, monkeypatch):
+    """A read-only data dir must degrade to a weaker salt, not to random-per-call.
+
+    Random per call is unbounded cardinality in PostHog, which is worse than no
+    salt at all.
+    """
+    telemetry._salt_cache = ""
+    monkeypatch.setattr(telemetry.os, "open", lambda *a, **k: (_ for _ in ()).throw(OSError("read-only")))
+    first = telemetry._install_salt()
+    telemetry._salt_cache = ""
+    assert telemetry._install_salt() == first