2c885fdcd7c246f59011ba517e35729bf6606a04
6 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Saket Aryan
|
2c885fdcd7 |
fix(plugins): make code.install reachable, and stop pinging on every flush
Review found the headline fix inverted: code.install could never fire, so every fresh install reported an upgrade and the two cohorts became indistinguishable — strictly worse than the bug being fixed. hook_runner reaches claim_install() only after cache_plugin_api_key() has written `api-key` and EvidenceStore() has created `evidence.sqlite3` and its WAL files. Asking "is the data directory empty" at that point always saw content. The caller now snapshots emptiness at the top of the run, before anything writes, and passes it in. Also caught by review, all in the same file: - claim_version_change was an unsynchronized read-modify-write, so several concurrently starting sessions each observed the old version and each recorded an upgrade. The first session after a version bump is exactly when a user's open agent windows all restart together. The transition is now claimed with an exclusive per-version sentinel. - A crash between O_EXCL and the write left an empty marker, which disabled every future upgrade event on that machine: claim_install saw the file and claim_version_change could not parse it. An unparseable marker is now repaired. - claim_install consumed the one-shot claim even under MEM0_TELEMETRY=false, so a user who opted out for their first sessions would never report install after opting in. - Existing users have an email but no key fingerprint, so the fast path always missed and every flush paid an uncached /v1/ping/ — a 5s timeout each time for the offline users this stack keeps citing. Legacy rows now adopt the current key's fingerprint instead of re-resolving. - A key that will not resolve (revoked, offline) kept attributing to the previous account's email, which is the bug this was meant to fix. It now falls back to the anonymous id. - The anonymous id was never rotated, so once it had been merged into one account it was still offered as the alias for the next one. An alias naming an already-identified id is what could link two real people; it is now offered once. The gap that let this ship was that no test drove hook_runner's session-start path — the decision was only ever tested by calling claim_install() directly on a directory nothing had touched. Adds subprocess tests that run the real entrypoint: fresh install, exactly-once, and an existing data dir. 62 core tests, 203 host tests. Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb |
||
|
Saket Aryan
|
0d2b20c03d |
fix(plugins): count installs once, and re-resolve the email when the key changes
code.install counted upgrades and repeat sessions. Session start records install whenever is_first_run() is true, and that only checked whether telemetry-identity.json exists. Recording install does not create that file — only the first successful flush does. So install fired for every 0.2.x user on their first 0.3.x session (0.2.x never wrote the file, and the data directory survives the upgrade), again for any session starting before that first flush finished, and — this is the part that makes it unbounded rather than a race — on every single session, forever, for anyone whose flush never succeeds. An offline or firewalled user reported a new install every time they opened an editor, which is exactly the population hardest to see in the data. A dedicated install-state.json is now claimed with O_CREAT|O_EXCL at the moment install is recorded, so two sessions starting together cannot both win, and the marker is not coupled to identity. Deliberately not the identity file: writing that from a recording process would race the sender, which writes it during resolve_distinct_id, and overloading it is what caused this. Upgrade detection keys on the data directory already having content. A fresh install has an empty one; anything else predates this session. That is a firmer predicate than looking for 0.2.x's venv/ and requirements.txt, which is a guess about files another part of the plugin may or may not have written and only ever works for this one upgrade. The version is stored in the marker so later changes record code.upgrade with a real from_version. A cached email outlived an API key change. resolve_distinct_id kept the first email it resolved and never looked again, so switching to a key from another account kept attributing events to the previous one. It now stores a fingerprint of the key the email came from and re-resolves when the current key differs, and falls back to the anonymous id when no key is configured rather than continuing to attribute to an account it cannot verify. The dangerous part is the alias. resolve_distinct_id's second return value becomes a PostHog $identify with $anon_distinct_id, and aliasing one account email to another merges two real person profiles irreversibly. The re-resolve path returns no alias; aliasing runs anonymous to email only, and never email to email. One existing test asserted that is_first_run flips when the identity file is written, which is the defect itself. Rewritten, along with coverage for atomic claiming, upgrade detection and version changes. Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb |
||
|
Saket Aryan
|
cfdfe40e09 |
fix(plugins): stop delivering telemetry events twice, and stop losing parked ones
Two defects, one cause: the spool protocol infers ownership instead of holding it, and never records progress. Duplicate delivery after a partial failure. flush() posts the claim in batches of 100 and returns on the first failure, keeping the whole file. The retry then posts every batch again, including the ones that already arrived — 150 recorded events were delivered 250 times. Progress is now written back to the claim after each successful batch, so a retry resumes where the send stopped and a crash repeats at most one batch. Duplicate delivery when two senders overlap. spool.replace(claim) is os.rename, which preserves mtime, so a claim created after a quiet minute inherited the spool's last-write time and looked abandoned the instant it existed. A second sender starting while the first was still posting took it over and sent it too — most likely at session end, when the MCP server's exit sender and the SessionEnd flush worker both drain. Claims are now touched at claim time, and the per-batch rewrite doubles as a lease heartbeat. _post makes one attempt with SEND_TIMEOUT and no retry, so a heartbeat lands well inside the 120s lease; a test asserts that margin so adding a retry loop to _post cannot silently break it. Parked batches starved. _claim_spool only looked at parked .sending files when no spool existed, and because sessions keep recording there usually was one — so a batch parked by a failed send waited until the 7-day expiry deleted it unsent, despite its own presence being what starts the sender in the first place. flush() now drains the live spool and then parked claims in the same run, oldest first, bounded. Expiry applies only after a genuine retry has failed, with the attempt count carried in the filename. A sender that gives up releases its lease rather than heartbeating on the way out, so the next run picks the batch up promptly instead of waiting a full stale window for a batch nobody is working on. A failing send stops the run, so one broken connection cannot burn every parked batch's attempt budget at once. Two existing tests asserted the old lifecycle and are updated in place, each with a comment saying what changed. Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb |
||
|
Saket Aryan
|
f9c566aa16 |
fix(plugins): report the plugin that produced the event, not the one that sent it
harness is set when an event is recorded; source was set when its batch was sent. Both came from module globals that stay at "generic" and "MEM0_PLUGIN" until telemetry.init() runs, and two processes in the pipeline never run it: - `python3 telemetry.py`, the detached sender spawn_flush() starts at session start, after every skill command, and when the MCP server exits. Everything it delivered was labelled source=MEM0_PLUGIN. Only batches flush_worker.py happened to drain got the real host. - mcp_server.py, which records every manual search as harness=generic. All six Python plugins ship the same files, so source could not tell any of them apart and MCP searches from every plugin landed in one generic bucket. The portable plugin is worse: it has no flush_worker at all, so its only sender is the uninitialised one and 100% of its events were mislabelled. Two changes. record() stamps source beside harness, so the sending process stops mattering — flush() already spreads per-event properties last, so a per-event source wins over any sender default. And the build generates core/_harness_id.py per host, seeding both modules at import, so identity no longer depends on an entrypoint remembering to call init(). The build already computed HARNESS_ID and spent it only on skill templating, and bundle_drift already diffs core/ byte-for-byte, so --check catches drift for free. Deliberately not adding MEM0_PLUGIN_HARNESS to the six manifests: they sit outside the --sync and --check boundary, which is the property that caused this. Also unifies two defaults that disagreed. configure_harness derived `<host>_plugin` while telemetry.init derived `MEM0_<HOST>_PLUGIN`, so a third value existed. It was unreachable only because hook_runner never calls flush(); moving source into record() would have made it live. Events now carry a uuid so a resend can be collapsed. The suite stayed green through all of this because the only tests live under one host, behind a conftest that calls init() at import. New tests run in real subprocesses with no init, and cover the portable plugin, which would pass a native-only test vacuously. Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb |
||
|
Saket Aryan
|
0d37619f24 |
fix(plugins): say what telemetry actually sends, and salt the hashes
The plugin README promises "anonymous usage events" and the telemetry module's docstring says it sends only "salted hashes". Neither is true. resolve_distinct_id() exchanges the API key for the account email and sends that as the distinct_id on every event. Installing the plugin requires an API key, so this is nearly every user. That is probably the behaviour we want — the Python SDK and the CLI attribute the same way — but the description has to match it. repo_hash and session_hash were unsalted SHA-256 cut to 16 hex characters. repo.identity is a git remote URL, or `local:<absolute path>` when there is no remote, which normally contains the account username. Sixteen unsalted hex characters over that input space is enumerable, so the hash was not a privacy control at all. Salted per install, with the salt kept in the identity file. That preserves every within-account join the analytics actually use and gives up only cross-machine joins on the same repository, which nothing computes. Since the distinct_id is already the email, the hash was never buying privacy from us — only from whoever obtains the data later, which is exactly what the salt fixes. Also corrects deepseek-plugin's README and source comment, which told readers ZAPIER and STRANDS were already in the backend's KNOWN_EVENT_SOURCES allowlist. Neither was. Adds a Telemetry section to docs/integrations/claude-code.mdx, which had none. Claude-Session: https://claude.ai/code/session_01C7tEmH86HAr7GoAAKCEHZb |
||
|
Kartik
|
73e7b8763a | refactor(integrations): shared agent plugin runtimes and native adapters (#7203) |