crypto: auto-heal split-brain device_keys state on for_user #88

Merged
jlxq0 merged 3 commits from crypto/auto-heal-device-keys into main 2026-08-29 12:46:03 +00:00
Owner

Root cause

matrix-rust-sdk's per-account shared flag (a.k.a. device_keys_published) latches true on the first successful /keys/upload and is never re-verified against the homeserver. If the upload was actually rejected — e.g. /setup attempting its own unbound bearer, a transient network blip, or a later admin-side device wipe — the SDK keeps operating against a device that doesn't exist server-side. Every subsequent /keys/upload is short-circuited (keys_for_upload() returns None while shared == true); the device shows up "Unverified — doesn't support encryption" in Element even though OTKs are uploaded; /setup/recover's self-sign attaches to local keys that never landed on Synapse.

Repro'd on 2026-05-21: MATRIXMCP2 had 50 OTKs in e2e_one_time_keys_json and zero rows in e2e_device_keys_json. matrix-rust-sdk emitted the canonical log line "Our own device might have been deleted" on every sync but no remediation triggered.

What this PR does

MatrixClientCache::for_user now post-verifies (Client::encryption().get_user_devices(...)) once per cached client. On a negative result it evicts, wipes the per-MXID store subdirectory, and recurses exactly once. A second negative after rebuild surfaces as an error rather than looping. The wipe is scoped to one MXID's hash directory under the store root — never the root itself, never another user's data.

  • src/matrix_client.rs: refactor for_userfor_user_inner(retry: bool), new helpers verify_or_heal and verify_device_keys_published, new wipe_user_store on the cache, new keys_verified: AtomicBool on CachedClient. New tests for the wipe (scope + idempotency) and the metric.
  • src/metrics.rs: new matrix_mcp_client_cache_self_heal_total{reason="..."} IntCounterVec. Single reason label for now (device_keys_missing); the design leaves room for future heal causes.
  • src/setup.rs: comment refresh — the existing contains() precondition stays as defense-in-depth; for_user is now the primary guard.

Why scoped per-MXID wipe is the right level

  • The split-brain is fundamentally per-user crypto state; nothing else in the store should be reset.
  • matrix-rust-sdk exposes no public way to flip shared back to false. The pickled Account is a single encrypted blob in kv[key="account"]; surgical mutation would require per-SDK-version schema understanding and is more fragile than a scoped rebuild.
  • Key backup restores Megolm history within seconds (~12s for 924 keys / 31 rooms in our repro). Olm sessions re-establish lazily and transparently. From the user's perspective /setup/recover just takes 30s longer; no error, no manual intervention.

Why not upstream this to matrix-rust-sdk

A public Encryption::republish_device_keys() or periodic verification of the shared flag against /keys/query would make this workaround vestigial. Worth filing eventually; not in scope for this PR.

Observability

Paired with argocd PR #551 which adds a Grafana Loki alert on the SDK warning "Our own device might have been deleted". The combination is: alert fires when the split-brain happens in the wild → the next /mcp tool call for that user auto-heals → metric increments → alert resolves on its own.

🤖 Generated with Claude Code

## Root cause matrix-rust-sdk's per-account `shared` flag (a.k.a. `device_keys_published`) latches `true` on the first successful `/keys/upload` and is never re-verified against the homeserver. If the upload was actually rejected — e.g. /setup attempting its own unbound bearer, a transient network blip, or a later admin-side device wipe — the SDK keeps operating against a device that doesn't exist server-side. Every subsequent `/keys/upload` is short-circuited (`keys_for_upload()` returns `None` while `shared == true`); the device shows up "Unverified — doesn't support encryption" in Element even though OTKs are uploaded; `/setup/recover`'s self-sign attaches to local keys that never landed on Synapse. Repro'd on 2026-05-21: `MATRIXMCP2` had 50 OTKs in `e2e_one_time_keys_json` and zero rows in `e2e_device_keys_json`. matrix-rust-sdk emitted the canonical log line `"Our own device might have been deleted"` on every sync but no remediation triggered. ## What this PR does `MatrixClientCache::for_user` now post-verifies (`Client::encryption().get_user_devices(...)`) once per cached client. On a negative result it evicts, wipes the per-MXID store subdirectory, and recurses exactly once. A second negative after rebuild surfaces as an error rather than looping. The wipe is scoped to one MXID's hash directory under the store root — never the root itself, never another user's data. - `src/matrix_client.rs`: refactor `for_user` → `for_user_inner(retry: bool)`, new helpers `verify_or_heal` and `verify_device_keys_published`, new `wipe_user_store` on the cache, new `keys_verified: AtomicBool` on `CachedClient`. New tests for the wipe (scope + idempotency) and the metric. - `src/metrics.rs`: new `matrix_mcp_client_cache_self_heal_total{reason="..."}` `IntCounterVec`. Single reason label for now (`device_keys_missing`); the design leaves room for future heal causes. - `src/setup.rs`: comment refresh — the existing `contains()` precondition stays as defense-in-depth; for_user is now the primary guard. ## Why scoped per-MXID wipe is the right level - The split-brain is fundamentally per-user crypto state; nothing else in the store should be reset. - matrix-rust-sdk exposes no public way to flip `shared` back to `false`. The pickled `Account` is a single encrypted blob in `kv[key="account"]`; surgical mutation would require per-SDK-version schema understanding and is more fragile than a scoped rebuild. - Key backup restores Megolm history within seconds (~12s for 924 keys / 31 rooms in our repro). Olm sessions re-establish lazily and transparently. From the user's perspective `/setup/recover` just takes 30s longer; no error, no manual intervention. ## Why not upstream this to matrix-rust-sdk A public `Encryption::republish_device_keys()` or periodic verification of the `shared` flag against `/keys/query` would make this workaround vestigial. Worth filing eventually; not in scope for this PR. ## Observability Paired with [argocd PR #551](https://github.com/thehansogroup/argocd/pull/551) which adds a Grafana Loki alert on the SDK warning `"Our own device might have been deleted"`. The combination is: alert fires when the split-brain happens in the wild → the next /mcp tool call for that user auto-heals → metric increments → alert resolves on its own. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
crypto: auto-heal split-brain device_keys state on for_user
All checks were successful
CI / cargo (pull_request) Successful in 45s
CI / docker (pull_request) Successful in 1m7s
4c55886cca
When matrix-rust-sdk's local `device_keys_published` flag is latched to
true but Synapse doesn't actually have our device's keys (e.g. an earlier
/keys/upload was rejected with the SDK still flipping the flag),
subsequent /keys/upload calls are skipped forever and the device stays
invisible to the homeserver. The SDK exposes no public API to clear the
flag, so the only recovery is to wipe the per-MXID crypto store and let
the SDK rebuild from scratch.

`MatrixClientCache::for_user` now post-verifies via /keys/query once per
cached client. On a negative result it evicts, wipes the per-MXID store
directory, and recurses exactly once. A second negative after rebuild
errors out instead of looping. The wipe is scoped to one MXID's
subdirectory under the store root — never the root itself, never
another user's data.

A new `matrix_mcp_client_cache_self_heal_total{reason}` counter records
each self-heal so we can alert on it in Loki/Grafana. The /setup
precondition stays as defense-in-depth.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Author
Owner

The pre-registered test hits: this is #88, not contention

Read 2026-08-29T06:1x. The prediction was named before looking — one-time keys present with device keys absent is #88's signature — and either outcome was informative. It came back positive, with the exact repro count.

MATRIXMCP-DLX8SKHK (unsigned)     one_time_keys 50   fallback 1   device_keys 0
MATRIXMCP-KZ9M8ZY8 (signed)       one_time_keys 21                device_keys 1

Fifty one-time keys and zero device-key rows is #88's repro exactly. The device is uploading keys — fifty of them — and none is a device key. The signed control has device keys and fewer OTKs, the healthy shape.

Why this ends the diagnosis where the other two did not

matrix-sdk-crypto-0.17.0/src/olm/account.rs:666: let device_keys = self.shared().not().then(|| self.device_keys()); — device keys are offered only while shared is false, and once it latches true keys_for_upload returns None for device keys forever while one-time keys keep flowing. So the device reaches this state with no upload being wrong at the moment it runs, because after the latch there is no device-key upload to be wrong.

That is independent of session count and of churn, which is why neither contention-by-count nor churn-recency explained it: eight-session KZ9 is fine, seven-session DLX8 with two quiescent windows is not, because the discriminator was never the sessions. It was whether shared had latched with device keys never persisted.

Contention-by-count and churn-recency were both post-hoc fits on n=2 and both died to a records read. This one named a falsifiable row-count pair first and survived it. #143, #130, #139 are #88.

What this does not settle

The fix is the lead's and is upstream in the SDK's latch behaviour, not in a session count. Whether the seven sessions should still be reaped is a separate hygiene question and not the cause. No revoke, no re-add, no code — records only.

## The pre-registered test hits: this is #88, not contention Read 2026-08-29T06:1x. The prediction was named before looking — one-time keys present with device keys absent is `#88`'s signature — and either outcome was informative. It came back positive, with the exact repro count. MATRIXMCP-DLX8SKHK (unsigned) one_time_keys 50 fallback 1 device_keys 0 MATRIXMCP-KZ9M8ZY8 (signed) one_time_keys 21 device_keys 1 **Fifty one-time keys and zero device-key rows is `#88`'s repro exactly.** The device is uploading keys — fifty of them — and none is a device key. The signed control has device keys and fewer OTKs, the healthy shape. ## Why this ends the diagnosis where the other two did not `matrix-sdk-crypto-0.17.0/src/olm/account.rs:666`: `let device_keys = self.shared().not().then(|| self.device_keys());` — device keys are offered only while `shared` is false, and once it latches true `keys_for_upload` returns `None` for device keys forever while one-time keys keep flowing. **So the device reaches this state with no upload being wrong at the moment it runs, because after the latch there is no device-key upload to be wrong.** That is independent of session count and of churn, which is why neither `contention-by-count` nor `churn-recency` explained it: eight-session KZ9 is fine, seven-session DLX8 with two quiescent windows is not, because the discriminator was never the sessions. It was whether `shared` had latched with device keys never persisted. **Contention-by-count and churn-recency were both post-hoc fits on n=2 and both died to a records read. This one named a falsifiable row-count pair first and survived it.** `#143`, `#130`, `#139` are `#88`. ## What this does not settle The fix is the lead's and is upstream in the SDK's latch behaviour, not in a session count. Whether the seven sessions should still be reaped is a separate hygiene question and not the cause. No revoke, no re-add, no code — records only.
Author
Owner

This cannot detect the state it exists for

Read against the current measurement: MATRIXMCP-DLX8SKHK has e2e_device_keys_json rows = 0 on Synapse today, after a successful /setup/recover, which is this PR's target state.

The heal is right and the trigger cannot fire.

verify_device_keys_published reduces to:

let devices = client.encryption().get_user_devices(&user_id).await?;
Ok(devices.get(device_id).is_some())

Encryption::get_user_devices calls OlmMachine::get_user_devices, which is self.store().get_user_devices(user_id)the local crypto store, with no homeserver round trip. wait_if_user_pending waits for an in-flight query and does not start one.

And Store::get_user_devices carries this note in the SDK's own source:

Note

: This method will include our own device which is always present in the store.

The memory store's get_own_device states the same invariant as an assertion: .expect("Invalid state: Should always have a own device").

So for our own user and our own device, the predicate is invariantly true. It returns Ok(true) on a device Synapse has never held a key for, keys_verified latches, and the wipe-and-rebuild never runs. The check asks the local store whether we know about ourselves, which we always do.

That is why this could sit for three months looking correct: its negative branch is unreachable, so nothing it guards was ever exercised, and no test would catch it because the predicate needs a live homeserver disagreeing with a live store.

What a working predicate has to do

Ask the homeserver, then read. secret_store.rs:443-452 already shows the shape used elsewhere in the SDK:

let (request_id, request) = olm_machine.query_keys_for_users([olm_machine.user_id()]);
client.keys_query(&request_id, request.device_keys).await?;

Force the /keys/query, then check the store, and treat a device absent from the response as the negative. Reading the store without forcing the query answers a different question.

The rest of the PR stands and is the valuable half: evict, wipe the per-MXID store subdirectory, rebuild, recurse exactly once, error on a second negative. That resets shared to false so keys_for_upload offers device keys again, which is the actual repair for the latch at matrix-sdk-crypto-0.17.0/src/olm/account.rs:666.

The gap this leaves in the diagnosis

Fixing the trigger makes the heal reachable; it does not explain why the keys never uploaded in the first place. The latch gates device keys and not one-time keys, so a device can go on publishing OTKs forever while shared says the identity is done. What set shared on a device Synapse never keyed is still unmeasured, and a heal that fires on every rebuild without that answer will wipe a crypto store repeatedly rather than once.

Cost of the heal, which belongs beside the decision: wiping the per-MXID store discards olm and megolm state. The PR's own figure is ~12 s to restore 924 keys across 31 rooms from key backup, which is cheap when key backup is present and recoverable and is not free otherwise.

## This cannot detect the state it exists for Read against the current measurement: `MATRIXMCP-DLX8SKHK` has `e2e_device_keys_json rows = 0` on Synapse today, after a successful `/setup/recover`, which is this PR's target state. **The heal is right and the trigger cannot fire.** `verify_device_keys_published` reduces to: let devices = client.encryption().get_user_devices(&user_id).await?; Ok(devices.get(device_id).is_some()) `Encryption::get_user_devices` calls `OlmMachine::get_user_devices`, which is `self.store().get_user_devices(user_id)` — **the local crypto store, with no homeserver round trip.** `wait_if_user_pending` waits for an in-flight query and does not start one. And `Store::get_user_devices` carries this note in the SDK's own source: > *Note*: This method will include our own device which is always present in the store. The memory store's `get_own_device` states the same invariant as an assertion: `.expect("Invalid state: Should always have a own device")`. **So for our own user and our own device, the predicate is invariantly `true`.** It returns `Ok(true)` on a device Synapse has never held a key for, `keys_verified` latches, and the wipe-and-rebuild never runs. **The check asks the local store whether we know about ourselves, which we always do.** That is why this could sit for three months looking correct: **its negative branch is unreachable**, so nothing it guards was ever exercised, and no test would catch it because the predicate needs a live homeserver disagreeing with a live store. ## What a working predicate has to do **Ask the homeserver, then read.** `secret_store.rs:443-452` already shows the shape used elsewhere in the SDK: let (request_id, request) = olm_machine.query_keys_for_users([olm_machine.user_id()]); client.keys_query(&request_id, request.device_keys).await?; Force the `/keys/query`, then check the store, and treat a device absent from the **response** as the negative. Reading the store without forcing the query answers a different question. **The rest of the PR stands and is the valuable half**: evict, wipe the per-MXID store subdirectory, rebuild, recurse exactly once, error on a second negative. That resets `shared` to false so `keys_for_upload` offers device keys again, which is the actual repair for the latch at `matrix-sdk-crypto-0.17.0/src/olm/account.rs:666`. ## The gap this leaves in the diagnosis **Fixing the trigger makes the heal reachable; it does not explain why the keys never uploaded in the first place.** The latch gates device keys and not one-time keys, so a device can go on publishing OTKs forever while `shared` says the identity is done. **What set `shared` on a device Synapse never keyed is still unmeasured**, and a heal that fires on every rebuild without that answer will wipe a crypto store repeatedly rather than once. **Cost of the heal, which belongs beside the decision**: wiping the per-MXID store discards olm and megolm state. The PR's own figure is ~12 s to restore 924 keys across 31 rooms from key backup, which is cheap **when key backup is present and recoverable** and is not free otherwise.
fix(crypto): make the split-brain check able to return false
All checks were successful
CI / cargo (pull_request) Successful in 1m25s
CI / docker (pull_request) Has been skipped
c149d2206e
The verification this PR added could not detect the state it exists for.
verify_device_keys_published reduced to
get_user_devices(own_user).get(own_device_id).is_some(), and
Encryption::get_user_devices calls OlmMachine::get_user_devices, which is
self.store().get_user_devices with no homeserver round trip;
wait_if_user_pending waits for a query already in flight rather than starting
one. The SDK's own note on that function says our own device is always present
in the store, and the memory store asserts it. So the predicate was invariantly
true for our own device: it returned Ok(true) for a device Synapse has never
held a key for, keys_verified latched, and the wipe never ran.

Its negative branch was unreachable, which is why nothing it guards was ever
exercised and why no unit test could have caught it: the fault needs a live
homeserver disagreeing with a live store.

The function's own doc comment is the reason it looked correct. It asserted
that get_user_devices "also issues /keys/query to the homeserver for any user
with a pending key-query", which is not what the SDK does. That sentence is
replaced with what the code actually does, because a comment explaining why a
check works answers the question a reviewer would otherwise ask.

It now sends /keys/query and reads the response rather than the store, and
device_in_keys_response is a free function over that response so the judgement
is testable at all: absence of the user, and presence of the user without the
device, both read as not published, with a control for the published case so
the assertions cannot pass against a predicate that is invariantly false.

Two bounds, because what set the SDK's shared flag on a device the homeserver
never keyed is still unmeasured.

At most one heal per identity per process. A second demand is a warning and a
metric rather than a second wipe, so an unexplained recurrence surfaces as the
heal being needed twice instead of as a crypto store wiped in a loop. The set
is deliberately not persisted: it bounds a loop within one process, and a
restart is a fresh chance to heal a genuinely new occurrence.

And no wipe without a recoverable key backup, checked per identity immediately
before wiping rather than assumed fleet-wide, because it differs per bot and at
least one account here has no cross-signing keys at all. Wiping discards olm
and megolm state, which is cheap only because backup restores it; with nothing
to restore from, a heal that destroys history to clear an unverified state is
worse than the state.

The verdict is logged before the heal runs. Today's live state, a device the
homeserver holds no key for, is the only control that shows this predicate can
return false, and it exists once. The log line survives the wipe that follows.

Mutations: the claim inverted so every heal is allowed, the claim keyed on a
constant so one identity's heal denies another's, the response predicate forced
true, and the device check relaxed to the user check. Each reddens its own test
and nothing else.

Merged main in first; the branch was 82 commits behind.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEgtox6vnEkYPv9PrCCoYG
Author
Owner

What is unverified in this change

"The fixed predicate returns false against today's live state" is a claim, not a measurement.

Running it needs a matrix-sdk client for @julian on the pod, and there is none: sixteen introspects on the current pod, zero mcp.tool spans, no client-lifecycle line. So the predicate cannot be exercised against the one state that is known to be a true negative, MATRIXMCP-DLX8SKHK with e2e_device_keys_json rows = 0.

The info! verdict is what will show it on first contact, and until a line appears naming that mxid and device, this is untested against production.

The gap is smaller than it sounds. The unit cases include the mirror control: mutating device_in_keys_response to true and relaxing it to the user-level check both redden the test, so the predicate cannot be invariantly either. What is unverified is the round trip, not the judgement.

The verdict is emitted on every path, including the refusals

Confirmed by reading the branch rather than from memory. The order inside Ok(false) is:

info!  "device keys absent from the homeserver's /keys/query response"
if retry              -> Err
if !claim_heal        -> warn + refused_second_heal   -> return
if !key_backup        -> warn + refused_no_key_backup -> return
warn!  self-healing...

The verdict is the first statement, ahead of the retry error and both refusals. So an identity that never heals still reports its state, which matters most for the one account here with no cross-signing keys at all: it will be refused at the backup check and will still have said what the homeserver holds for it.

Blast radius, stated for the reviewer

If the predicate were wrong in the mirror direction, invariantly false, it would fire a heal that was not needed. The bounds cap that at one recoverable wipe per identity per process, and only where backups().exists_on_server() answers true immediately beforehand. An unnecessary wipe with a backup present costs ~12 s of key restore; the current state is a device that cannot decrypt at all.

## What is unverified in this change **"The fixed predicate returns false against today's live state" is a claim, not a measurement.** Running it needs a matrix-sdk client for `@julian` on the pod, and there is none: sixteen introspects on the current pod, zero `mcp.tool` spans, no client-lifecycle line. So the predicate cannot be exercised against the one state that is known to be a true negative, `MATRIXMCP-DLX8SKHK` with `e2e_device_keys_json rows = 0`. **The `info!` verdict is what will show it on first contact**, and until a line appears naming that mxid and device, this is untested against production. **The gap is smaller than it sounds.** The unit cases include the mirror control: mutating `device_in_keys_response` to `true` and relaxing it to the user-level check both redden the test, so the predicate cannot be invariantly either. What is unverified is the round trip, not the judgement. ## The verdict is emitted on every path, including the refusals Confirmed by reading the branch rather than from memory. The order inside `Ok(false)` is: info! "device keys absent from the homeserver's /keys/query response" if retry -> Err if !claim_heal -> warn + refused_second_heal -> return if !key_backup -> warn + refused_no_key_backup -> return warn! self-healing... **The verdict is the first statement**, ahead of the retry error and both refusals. So an identity that never heals still reports its state, which matters most for the one account here with no cross-signing keys at all: it will be refused at the backup check and will still have said what the homeserver holds for it. ## Blast radius, stated for the reviewer If the predicate were wrong in the mirror direction, invariantly false, it would fire a heal that was not needed. The bounds cap that at **one recoverable wipe per identity per process**, and only where `backups().exists_on_server()` answers true immediately beforehand. **An unnecessary wipe with a backup present costs ~12 s of key restore; the current state is a device that cannot decrypt at all.**
Author
Owner

Gated on the pinned toolchain and mutated on the predicate that was the bug. Merging.

cargo +1.98.0 fmt --check                                        rc=0
cargo +1.98.0 clippy --all-targets --all-features -D warnings    rc=0
cargo +1.98.0 test --all-features --locked                       rc=0, 308 passed

My mutation, relaxing the check to the user level, which is the shape the old bug had:

response.device_keys.get(user_id).is_some_and(|d| d.contains_key(device_id))
  ->  response.device_keys.contains_key(user_id)

FAIL  a_keys_query_that_omits_the_device_reads_as_not_published
307 passed, 1 failed

A response listing the user without our device now reads as published again, and exactly one case says so.

What was actually wrong, and why it survived three months

verify_device_keys_published reduced to get_user_devices(own_user).get(own_device_id).is_some(), which reads the local crypto store with no homeserver round trip. The SDK's own note says our own device is always present there, and the memory store asserts it. So the predicate was invariantly true for the case it was written to detect, returning Ok(true) for a device Synapse has never held a key for.

Its negative branch was unreachable, so nothing it guarded was ever exercised, and no unit test could reach it: it needs a live homeserver disagreeing with a live store.

And the doc comment said get_user_devices "also issues /keys/query to the homeserver for any user with a pending key-query". It does not. A reviewer asking how this reaches the server was answered by the comment and stopped, which is why the explanation is the thing that had to be wrong for the check to look right. Replaced rather than appended, so the next reader does not meet both readings.

The bounds, and why this ships without the shared question answered

One heal per identity per process, second demand a warning with refused_second_heal, the set unpersisted because it bounds a loop within a process and a restart is a fresh chance at a genuine occurrence.

No wipe without backups().exists_on_server(), checked per identity immediately before, with refused_no_key_backup. @lucybot has no cross-signing keys, so this is per-identity rather than fleet-wide by necessity.

Blast radius: one recoverable wipe per identity, only where a backup exists, against a device that currently cannot decrypt at all. Twelve seconds for 924 keys.

The verdict is logged at INFO first, ahead of the retry error and both refusals, so an identity that never heals still reports what the homeserver holds for it.

What is unverified and is stated on the PR

The fixed predicate has not run against today's live state, because it needs a client for @julian on the pod and there is none. The log line is what will show it on first contact. The gap is the round trip rather than the judgement: the mirror control means the predicate cannot be invariantly either.

And the question this does not answer

Nothing here explains what set shared on a device the homeserver never keyed. The bounds make the fix survivable without that answer rather than supplying it.

refused_second_heal on a fresh process is the signal that something re-creates the state, and if it appears the question becomes urgent again.

**Gated on the pinned toolchain and mutated on the predicate that was the bug. Merging.** cargo +1.98.0 fmt --check rc=0 cargo +1.98.0 clippy --all-targets --all-features -D warnings rc=0 cargo +1.98.0 test --all-features --locked rc=0, 308 passed **My mutation, relaxing the check to the user level, which is the shape the old bug had:** response.device_keys.get(user_id).is_some_and(|d| d.contains_key(device_id)) -> response.device_keys.contains_key(user_id) FAIL a_keys_query_that_omits_the_device_reads_as_not_published 307 passed, 1 failed **A response listing the user without our device now reads as published again, and exactly one case says so.** ## What was actually wrong, and why it survived three months **`verify_device_keys_published` reduced to `get_user_devices(own_user).get(own_device_id).is_some()`, which reads the local crypto store with no homeserver round trip.** The SDK's own note says our own device is always present there, and the memory store asserts it. **So the predicate was invariantly true for the case it was written to detect**, returning `Ok(true)` for a device Synapse has never held a key for. **Its negative branch was unreachable, so nothing it guarded was ever exercised**, and no unit test could reach it: it needs a live homeserver disagreeing with a live store. **And the doc comment said `get_user_devices` "also issues `/keys/query` to the homeserver for any user with a pending key-query".** It does not. **A reviewer asking how this reaches the server was answered by the comment and stopped**, which is why the explanation is the thing that had to be wrong for the check to look right. **Replaced rather than appended**, so the next reader does not meet both readings. ## The bounds, and why this ships without the `shared` question answered **One heal per identity per process**, second demand a warning with `refused_second_heal`, the set unpersisted because it bounds a loop within a process and a restart is a fresh chance at a genuine occurrence. **No wipe without `backups().exists_on_server()`**, checked per identity immediately before, with `refused_no_key_backup`. **`@lucybot` has no cross-signing keys, so this is per-identity rather than fleet-wide by necessity.** **Blast radius: one recoverable wipe per identity, only where a backup exists, against a device that currently cannot decrypt at all.** Twelve seconds for 924 keys. **The verdict is logged at INFO first**, ahead of the retry error and both refusals, so an identity that never heals still reports what the homeserver holds for it. ## What is unverified and is stated on the PR **The fixed predicate has not run against today's live state**, because it needs a client for `@julian` on the pod and there is none. **The log line is what will show it on first contact.** The gap is the round trip rather than the judgement: the mirror control means the predicate cannot be invariantly either. ## And the question this does not answer **Nothing here explains what set `shared` on a device the homeserver never keyed.** The bounds make the fix survivable without that answer rather than supplying it. **`refused_second_heal` on a fresh process is the signal that something re-creates the state**, and if it appears the question becomes urgent again.
jlxq0 merged commit 752e454a89 into main 2026-08-29 12:46:03 +00:00
jlxq0 deleted branch crypto/auto-heal-device-keys 2026-08-29 12:46:03 +00:00
Sign in to join this conversation.
No description provided.