fix(desktop): republish agent identity records when a persona rename propagates#2607
fix(desktop): republish agent identity records when a persona rename propagates#2607SeanGearin wants to merge 2 commits into
Conversation
…propagates Signed-off-by: Sean Gearin <sgearin@gmail.com>
81be1c0 to
63a53dc
Compare
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 This is a nice fix — the root cause is exactly right (update_persona's rename path propagates the new name and saves managed-agents.json but never re-retains the kind:30177 identity records, so the relay carries the stale name→pubkey binding until the next boot reconcile), and I like that the fix extracts the duplicated retain logic from retain_managed_agent_pending and reconcile_agents_in_dir into one shared retain_agent_record engine instead of adding a third copy — production code is net negative here. The two tests are discriminating (rename re-retains with a monotonic created_at bump past the retained head; unchanged record is a true no-op with no pending_sync churn), and the avatar-only exclusion is correct since the avatar isn't part of the published projection.
One blocking issue, and it's mechanical:
Desktop Core is red on the file-size ratchet. The check fails with src-tauri/src/commands/personas/mod.rs: 985 lines (limit 984) — that file has a TEMP per-file ceiling of exactly 984 in desktop/scripts/check-file-sizes.mjs, and this PR's 13 added lines in update_persona land it 1 line over. Rather than bumping the override (the policy comment in that script says not to), the easiest fix is to trim the comment above the new retain loop — it's 10 lines for a 3-line loop, and 2-3 of those lines (the #2423 reference and the avatar-exclusion rationale) carry all the content.
One non-blocking thought: the engine itself is well-tested, but nothing pins that update_persona actually calls it on a rename — a thin test at the propagation layer would close the #2423 path end-to-end and protect against a future refactor silently dropping the call. Fine as a follow-up.
Happy to approve once the size gate is green.
check-file-sizes reported personas/mod.rs at 985 against its 984 TEMP ceiling, so Desktop Core and Desktop were red. The script's own policy says not to add to the override list, so this trims the comment instead of raising the limit. The 10-line block above the retain loop is now 6 and keeps what carries the content: the block#2423 reference, that record.name is in the published identity projection, the stale-binding consequence, and why avatar-only edits are excluded. 981 lines; check-file-sizes passes locally. Signed-off-by: Sean Gearin <sgearin@gmail.com>
|
Fixed, and thanks for pointing at the comment rather than the override — the policy note in The block above the retain loop is 10 lines down to 6, keeping what actually carries weight: the #2423 reference, that On the propagation test — you are right that it is the real gap. The engine is covered and the call is not, so a refactor could silently drop it and every existing test would stay green. The awkwardness is that the natural home for it is Happy to file the issue so it does not get lost. Say the word if you would rather see it in this PR and I will find the lines elsewhere. |
Problem
Part of #2423 (renaming personal agents desynchronises identity).
Renaming an agent definition (persona) propagates the new display name to its
linked agent instances (
propagate_persona_name_renameindesktop/src-tauri/src/commands/personas/mod.rs) and savesmanaged-agents.json— but, unlike the instance-rename path(
update_managed_agent), it never re-retains the renamed instances' kind:30177managed-agent identity records.
record.nameis part of the published identityprojection (
agent_event_content), so after a persona rename:managed-agents.jsonsays the NEW name,the OLD name, with the OLD
created_at.The stale identity record stays live on the relay until the next app launch,
when the boot-time reconcile (
reconcile_agents_in_dir) finally notices thecontent diff and republishes. Until that restart, any surface that resolves
agents from kind:30177 records (second desktop of the same owner, CLI, other
NIP-AP clients) sees the OLD name bound to the agent pubkey while the kind:0
profile already shows the NEW one — the name→identity binding desync described
in #2423, and consistent with the report's observation that repairing state
required "a separate restart".
Fix
managed_agents::reconcile::retain_agent_record(conn, keys, record) -> Result<bool, String>— one shared content-diff + monotonic-
created_at-bump engine (returnswhether a row was rewritten).
reconcile_agents_in_dirnow calls it perrecord (behavior unchanged; existing reconcile tests still pass).
commands::agents::retain_managed_agent_pendingdelegates to the sharedengine instead of carrying a duplicate implementation (same semantics:
projection-equality no-op guard, monotonic bump,
pending_sync = 1).update_persona(Phase 1, still under the store lock, aftersave_managed_agents): callretain_managed_agent_pendingfor every recordthe rename propagated to — mirroring
update_managed_agent. Avatar-onlyedits are deliberately excluded (the avatar is not part of the kind:30177
projection; retaining would be a guaranteed no-op).
No new events, kinds, or APIs — this uses the existing signed-event retention
and flush pipeline, per CONTRIBUTING's guidance to prefer a signed Nostr event
and the existing ingest path over endpoint-specific JSON APIs.
Out of scope (deliberately)
territory (and largely superseded by the merged rollback in fix(desktop): refresh cached channel member names #2258).
repair for stale identities: TS-side, noted in [Bug] Renaming or re-adding personal agents can desynchronise identity and break @mentions #2423, not touched here.
Test evidence
Two new unit tests in
desktop/src-tauri/src/managed_agents/reconcile/tests.rs(same harness as the existing reconcile tests — tempdir + retention.db + fresh
keys, no AppHandle):
rename_re_retains_identity_record_with_new_name— retain "Fizz", confirmflush, rename to "Spark", re-retain: row keeps the pubkey coordinate,
carries the new name only, is
pending_sync, and itscreated_atisstrictly past the retained head (replaceable-event acceptance).
retain_agent_record_is_noop_when_unchanged— an unchanged projection doesnot rewrite the row and produces zero
pending_syncchurn.Ran scoped per CONTRIBUTING build discipline (from
desktop/src-tauri):Full
buzz-desktoplib suite: 1562 passed, 0 failed, 13 ignored —including all 12
managed_agents::reconciletests (10 pre-existing, allunmodified in behavior, plus the 2 new regression tests above).
Links
(kind:0 sync-failure surfacing), merged fix(desktop): refresh cached channel member names #2258 (instance-rename rollback).