mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): republish agent identity records when a persona rename propagates (#2607)
## 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_rename` in `desktop/src-tauri/src/commands/personas/mod.rs`) and saves `managed-agents.json` — but, unlike the instance-rename path (`update_managed_agent`), it never re-retains the renamed instances' kind:30177 managed-agent identity records. `record.name` is part of the published identity projection (`agent_event_content`), so after a persona rename: - `managed-agents.json` says the NEW name, - the retained kind:30177 row (retention.db → relay flush loop) still carries 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 the content 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 - Extract the per-record retain body of the boot reconcile into `managed_agents::reconcile::retain_agent_record(conn, keys, record) -> Result<bool, String>` — one shared content-diff + monotonic-`created_at`-bump engine (returns whether a row was rewritten). `reconcile_agents_in_dir` now calls it per record (behavior unchanged; existing reconcile tests still pass). - `commands::agents::retain_managed_agent_pending` delegates to the shared engine 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, after `save_managed_agents`): call `retain_managed_agent_pending` for every record the rename propagated to — mirroring `update_managed_agent`. Avatar-only edits 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) - Rename → runtime restart is #1823, fixed by open PR #2507 (spawn_hash). - Surfacing kind:0 relay profile-sync failures on rename is PR #2302/#2279 territory (and largely superseded by the merged rollback in #2258). - Mention-picker UX (owner/status disambiguation) and channel-membership repair for stale identities: TS-side, noted in #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", confirm flush, rename to "Spark", re-retain: row keeps the pubkey coordinate, carries the new name only, is `pending_sync`, and its `created_at` is strictly past the retained head (replaceable-event acceptance). - `retain_agent_record_is_noop_when_unchanged` — an unchanged projection does not rewrite the row and produces zero `pending_sync` churn. Ran scoped per CONTRIBUTING build discipline (from `desktop/src-tauri`): ``` cargo fmt -p buzz-desktop # applied, clean cargo clippy -p buzz-desktop --lib --tests -- -D warnings # exit 0, no warnings cargo test -p buzz-desktop --lib # full lib suite ``` Full `buzz-desktop` lib suite: **1562 passed, 0 failed, 13 ignored** — including all 12 `managed_agents::reconcile` tests (10 pre-existing, all unmodified in behavior, plus the 2 new regression tests above). ## Links - Issue: https://github.com/block/buzz/issues/2423 - Adjacent (no overlap): PR #2507 (rename-restart, #1823), PRs #2302/#2279 (kind:0 sync-failure surfacing), merged #2258 (instance-rename rollback). --------- Signed-off-by: Sean Gearin <sgearin@gmail.com> Co-authored-by: Sean Gearin <sgearin@gmail.com> Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
co-authored by
Sean Gearin
Will Pfleger
parent
de13960505
commit
7ca0bbd946
@@ -45,52 +45,15 @@ pub(super) fn retain_managed_agent_pending(
|
||||
state: &AppState,
|
||||
record: &ManagedAgentRecord,
|
||||
) {
|
||||
use crate::managed_agents::{
|
||||
agent_events::{agent_event_content, build_agent_event},
|
||||
persona_events::monotonic_created_at,
|
||||
retention::{get_retained_event, open_retention_db, retain_event, RetainedEvent},
|
||||
};
|
||||
use buzz_core_pkg::kind::KIND_MANAGED_AGENT;
|
||||
use nostr::JsonUtil;
|
||||
use crate::managed_agents::{reconcile::retain_agent_record, retention::open_retention_db};
|
||||
|
||||
let result = (|| -> Result<(), String> {
|
||||
let conn = open_retention_db(&managed_agents_base_dir(app)?.join("retention.db"))?;
|
||||
// The published content is the opt-IN projection JSON, independent of
|
||||
// signing and created_at. Compute it once to drive the no-republish
|
||||
// guard without signing twice.
|
||||
let content = serde_json::to_string(&agent_event_content(record))
|
||||
.map_err(|e| format!("failed to serialize managed-agent content: {e}"))?;
|
||||
let (owner_pubkey, event) = {
|
||||
let keys = state.signing_keys()?;
|
||||
let owner_pubkey = keys.public_key().to_hex();
|
||||
let existing =
|
||||
get_retained_event(&conn, KIND_MANAGED_AGENT, &owner_pubkey, &record.pubkey)?;
|
||||
// Skip re-publishing when the projection is unchanged: a start/stop
|
||||
// or any edit that touched only excluded runtime/local fields
|
||||
// produces an identical projection, so it is a no-op — operational
|
||||
// churn never re-enqueues a publish.
|
||||
if existing.as_ref().is_some_and(|row| row.content == content) {
|
||||
return Ok(());
|
||||
}
|
||||
// Monotonic created_at: bump past the retained head (NIP-AP step 3).
|
||||
let event = build_agent_event(record)?
|
||||
.custom_created_at(monotonic_created_at(existing.map(|row| row.created_at)))
|
||||
.sign_with_keys(&keys)
|
||||
.map_err(|e| format!("failed to sign managed-agent event: {e}"))?;
|
||||
(owner_pubkey, event)
|
||||
};
|
||||
retain_event(
|
||||
&conn,
|
||||
&RetainedEvent {
|
||||
kind: KIND_MANAGED_AGENT,
|
||||
pubkey: owner_pubkey,
|
||||
d_tag: record.pubkey.clone(),
|
||||
content: event.content.to_string(),
|
||||
created_at: event.created_at.as_secs() as i64,
|
||||
raw_event: event.as_json(),
|
||||
pending_sync: true,
|
||||
},
|
||||
)
|
||||
let keys = state.signing_keys()?;
|
||||
// Shared engine with the boot-time reconcile: projection content diff
|
||||
// (no republish for runtime-only churn) + monotonic created_at bump
|
||||
// past the retained head (NIP-AP step 3).
|
||||
retain_agent_record(&conn, &keys, record).map(|_| ())
|
||||
})();
|
||||
if let Err(e) = result {
|
||||
eprintln!("buzz-desktop: agent-retain: {e}");
|
||||
|
||||
@@ -272,6 +272,15 @@ pub async fn update_persona(
|
||||
|
||||
if agents_modified {
|
||||
save_managed_agents(&app, &records)?;
|
||||
// Keep retained kind:30177 identity records in lockstep with
|
||||
// the rename (#2423): `record.name` is part of the published
|
||||
// identity projection, so skipping this strands the relay on
|
||||
// the stale name→pubkey binding until the next boot reconcile.
|
||||
// Avatar-only edits are excluded — the avatar is not in the
|
||||
// projection, so retaining would be a guaranteed no-op.
|
||||
for record in records.iter().filter(|r| renamed.contains(&r.pubkey)) {
|
||||
super::agents::retain_managed_agent_pending(&app, &state, record);
|
||||
}
|
||||
}
|
||||
|
||||
params
|
||||
|
||||
@@ -79,8 +79,6 @@ pub(crate) fn reconcile_agents_in_dir(base_dir: &Path, keys: &nostr::Keys) -> Re
|
||||
return Ok(0);
|
||||
}
|
||||
|
||||
let owner_pubkey = keys.public_key().to_hex();
|
||||
|
||||
let db_path = base_dir.join("retention.db");
|
||||
let conn =
|
||||
open_retention_db(&db_path).map_err(|e| format!("failed to open retention db: {e}"))?;
|
||||
@@ -94,46 +92,66 @@ pub(crate) fn reconcile_agents_in_dir(base_dir: &Path, keys: &nostr::Keys) -> Re
|
||||
continue;
|
||||
}
|
||||
|
||||
let existing =
|
||||
get_retained_event(&conn, KIND_MANAGED_AGENT, &owner_pubkey, &record.pubkey)?;
|
||||
|
||||
// Build the event first and compare ITS content, so the comparison and
|
||||
// the retained row share one serialization of the projection (mirrors
|
||||
// `migrate_personas_in_dir`). Serializing the projection independently
|
||||
// here would silently diverge if `build_agent_event` ever changed how
|
||||
// it serializes — republishing every agent every boot. Content is
|
||||
// timestamp-independent, so the monotonic bump below never forces a
|
||||
// spurious republish; an unchanged agent is still a true no-op.
|
||||
let event = build_agent_event(record)?
|
||||
.custom_created_at(monotonic_created_at(
|
||||
existing.as_ref().map(|row| row.created_at),
|
||||
))
|
||||
.sign_with_keys(keys)
|
||||
.map_err(|e| format!("failed to sign event for '{}': {e}", record.name))?;
|
||||
|
||||
let content = event.content.clone();
|
||||
if existing.as_ref().is_some_and(|row| row.content == content) {
|
||||
continue;
|
||||
if retain_agent_record(&conn, keys, record)? {
|
||||
reconciled += 1;
|
||||
}
|
||||
|
||||
retain_event(
|
||||
&conn,
|
||||
&RetainedEvent {
|
||||
kind: KIND_MANAGED_AGENT,
|
||||
pubkey: owner_pubkey.clone(),
|
||||
d_tag: record.pubkey.clone(),
|
||||
content,
|
||||
created_at: event.created_at.as_secs() as i64,
|
||||
raw_event: event.as_json(),
|
||||
pending_sync: true,
|
||||
},
|
||||
)
|
||||
.map_err(|e| format!("failed to retain '{}': {e}", record.name))?;
|
||||
reconciled += 1;
|
||||
}
|
||||
|
||||
Ok(reconciled)
|
||||
}
|
||||
|
||||
/// Retain `record`'s kind:30177 identity record, marking it `pending_sync`
|
||||
/// for the flush loop, when its projection differs from the retained head.
|
||||
/// Returns `Ok(true)` when a row was (re)written and `Ok(false)` when the
|
||||
/// retained content already matches (a true no-op — no `pending_sync` churn).
|
||||
///
|
||||
/// This is the single content-diff + monotonic-bump engine shared by the
|
||||
/// boot-time reconcile above and the interactive edit paths
|
||||
/// (`retain_managed_agent_pending`, persona-rename propagation). Every
|
||||
/// mutation of an agent's published identity must go through it so the
|
||||
/// retained record can never silently drift from `managed-agents.json`.
|
||||
pub(crate) fn retain_agent_record(
|
||||
conn: &rusqlite::Connection,
|
||||
keys: &nostr::Keys,
|
||||
record: &ManagedAgentRecord,
|
||||
) -> Result<bool, String> {
|
||||
let owner_pubkey = keys.public_key().to_hex();
|
||||
let existing = get_retained_event(conn, KIND_MANAGED_AGENT, &owner_pubkey, &record.pubkey)?;
|
||||
|
||||
// Build the event first and compare ITS content, so the comparison and
|
||||
// the retained row share one serialization of the projection (mirrors
|
||||
// `migrate_personas_in_dir`). Serializing the projection independently
|
||||
// here would silently diverge if `build_agent_event` ever changed how
|
||||
// it serializes — republishing every agent every boot. Content is
|
||||
// timestamp-independent, so the monotonic bump below never forces a
|
||||
// spurious republish; an unchanged agent is still a true no-op.
|
||||
let event = build_agent_event(record)?
|
||||
.custom_created_at(monotonic_created_at(
|
||||
existing.as_ref().map(|row| row.created_at),
|
||||
))
|
||||
.sign_with_keys(keys)
|
||||
.map_err(|e| format!("failed to sign event for '{}': {e}", record.name))?;
|
||||
|
||||
let content = event.content.clone();
|
||||
if existing.as_ref().is_some_and(|row| row.content == content) {
|
||||
return Ok(false);
|
||||
}
|
||||
|
||||
retain_event(
|
||||
conn,
|
||||
&RetainedEvent {
|
||||
kind: KIND_MANAGED_AGENT,
|
||||
pubkey: owner_pubkey,
|
||||
d_tag: record.pubkey.clone(),
|
||||
content,
|
||||
created_at: event.created_at.as_secs() as i64,
|
||||
raw_event: event.as_json(),
|
||||
pending_sync: true,
|
||||
},
|
||||
)
|
||||
.map_err(|e| format!("failed to retain '{}': {e}", record.name))?;
|
||||
Ok(true)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests;
|
||||
|
||||
@@ -300,3 +300,103 @@ fn slimming_republish_wave_is_one_time() {
|
||||
"second boot must be a no-op (idempotence)"
|
||||
);
|
||||
}
|
||||
|
||||
// ── retain_agent_record (interactive-edit engine) ────────────────────────────
|
||||
//
|
||||
// #2423: renaming an agent must re-retain its kind:30177 identity record
|
||||
// IMMEDIATELY, not at the next boot-time reconcile. These tests pin the shared
|
||||
// engine both the boot reconcile and the interactive edit paths
|
||||
// (`retain_managed_agent_pending`, persona-rename propagation) run on.
|
||||
|
||||
/// A rename re-retains the identity record under the SAME coordinate (the
|
||||
/// agent pubkey) with the new name, queued for publish, with a created_at
|
||||
/// strictly past the retained head so the relay's replaceable-event rule
|
||||
/// accepts it. Without this, the relay keeps the old name→pubkey binding
|
||||
/// until the next restart — the identity desync in #2423.
|
||||
#[test]
|
||||
fn rename_re_retains_identity_record_with_new_name() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let keys = nostr::Keys::generate();
|
||||
let conn = open_retention_db(&dir.path().join("retention.db")).unwrap();
|
||||
let owner = keys.public_key().to_hex();
|
||||
let pubkey = "9".repeat(64);
|
||||
let mut record = sample_record(&pubkey, "Fizz");
|
||||
|
||||
assert!(retain_agent_record(&conn, &keys, &record).unwrap());
|
||||
let first = get_retained_event(&conn, KIND_MANAGED_AGENT, &owner, &pubkey)
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
// Simulate the flush loop confirming the initial publish.
|
||||
mark_synced(
|
||||
&conn,
|
||||
first.kind,
|
||||
&first.pubkey,
|
||||
&first.d_tag,
|
||||
first.created_at,
|
||||
&first.content,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
record.name = "Spark".to_string();
|
||||
assert!(
|
||||
retain_agent_record(&conn, &keys, &record).unwrap(),
|
||||
"a renamed record must re-retain its identity record"
|
||||
);
|
||||
|
||||
let row = get_retained_event(&conn, KIND_MANAGED_AGENT, &owner, &pubkey)
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
assert_eq!(row.d_tag, pubkey, "coordinate stays keyed by agent pubkey");
|
||||
assert!(
|
||||
row.content.contains("Spark"),
|
||||
"retained identity record must carry the new name"
|
||||
);
|
||||
assert!(
|
||||
!row.content.contains("Fizz"),
|
||||
"the stale name must not survive the rename"
|
||||
);
|
||||
assert!(row.pending_sync, "a rename must queue a republish");
|
||||
assert!(
|
||||
row.created_at > first.created_at,
|
||||
"created_at must bump past the retained head (replaceable-event rule)"
|
||||
);
|
||||
}
|
||||
|
||||
/// An unchanged record is a true no-op: no rewrite, no `pending_sync` churn.
|
||||
/// This is what lets every edit path call the engine unconditionally.
|
||||
#[test]
|
||||
fn retain_agent_record_is_noop_when_unchanged() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let keys = nostr::Keys::generate();
|
||||
let conn = open_retention_db(&dir.path().join("retention.db")).unwrap();
|
||||
let pubkey = "8".repeat(64);
|
||||
let record = sample_record(&pubkey, "steady-agent");
|
||||
|
||||
assert!(retain_agent_record(&conn, &keys, &record).unwrap());
|
||||
let row = get_retained_event(
|
||||
&conn,
|
||||
KIND_MANAGED_AGENT,
|
||||
&keys.public_key().to_hex(),
|
||||
&pubkey,
|
||||
)
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
mark_synced(
|
||||
&conn,
|
||||
row.kind,
|
||||
&row.pubkey,
|
||||
&row.d_tag,
|
||||
row.created_at,
|
||||
&row.content,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert!(
|
||||
!retain_agent_record(&conn, &keys, &record).unwrap(),
|
||||
"an unchanged projection must not re-retain"
|
||||
);
|
||||
assert!(
|
||||
get_pending_sync(&conn).unwrap().is_empty(),
|
||||
"no pending_sync churn for an unchanged record"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user