From aa39d72aac57aeda49cb3c38db7c9c8ed4af24f1 Mon Sep 17 00:00:00 2001 From: Sami Date: Thu, 6 Aug 2026 13:19:49 -0400 Subject: [PATCH] fix(agents): stop boot reconcile republishing stale config over a newer head Boot reconcile is a fourth stale-disk republish site, in the same class as the three write sites fixed in the previous commit but worse: it fires at launch, unprompted, for every agent on a device that follows another device's config. `reconcile_agents_in_dir_at` reads `managed-agents.json` raw and cannot resolve the private-config overlay -- `hydrate_private_config_overlay` runs after this leg (`event_sync.rs:19-20`) and reads the rows this leg writes. Inbound kind:30179 updates the overlay and retention but never the JSON, so on a follower disk is stale by construction. Rebuilding the 30179 projection from disk then republishes every stale field over device A's newer head as a validly chained gen+1 successor, and `monotonic_created_at` floors it at head+1 so it wins LWW. Measured: gen 5 -> 6, `prev` = the clobbered head, `created_at` = head+1 against a head 10,000s in the future, `pending_sync` set, and every field (name, system_prompt, parallelism, env_vars) taken from stale disk. It also does not self-heal. A second boot is a clean no-op because disk now matches the head it wrote, but each new head device A publishes re-arms it: measured 16 -> 1, no-op, then 24 -> 1. The follower's disk wins every round and the user on A sees their edit silently revert. The previous commit's keyring hydration is what makes this reachable. Before it, `retain_private_agent_record`'s empty-nsec skip returned early for every keyring-resident record, so boot never built a 30179 at all -- the skip was incidentally protecting this path. Hydrating keys is still correct (an untouched agent must publish its first 30179 on a default build), but it exposed everything downstream of the guard it removed. A control arm with an absent nsec confirms the head survives, pinning the causal line. Fix: `retain_agent_record_at_boot` publishes the 30179 only when no retained head exists, and is used by boot reconcile alone. That keeps the requirement boot exists to serve -- an agent whose nsec lives in the keyring gets its FIRST private config published -- while leaving an existing head to the interactive edit paths, which resolve the overlay before retaining and so author from relay-fresh state. The kind:30177 identity leg is untouched, so the upgrade republish waves keep working. Resolving the overlay at boot instead was rejected and is pinned by a permanent wrong-fix probe: an offline local edit lives on disk and in an unflushed `pending_sync` 30179, so resolving disk through an overlay hydrated from the older head would discard it -- the centralized-resolve failure from the previous commit, with boot's blast radius. Tests (4): the fix verification asserts the head is byte-identical after boot and nothing is enqueued; two requirement-preservation arms (first 30179 still published when no head exists; 30177 still republishes when a private head is present) so the fix cannot be satisfied by never publishing at boot or by gating at the wrong level; and the wrong-fix probe. Three mutants, each killed by a different arm: gate deleted, gate inverted, gate applied to the whole record instead of the private leg. Mutants re-run after cargo fmt. Desktop lib suite 2360 passed / 0 failed / 15 ignored (--all-features); cargo fmt --check, cargo clippy --workspace --all-targets --all-features -D warnings, and the desktop file-size ratchet (against the CI base) all clean -- the ratchet verified live with a padding control that fails it. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- .../src-tauri/src/managed_agents/reconcile.rs | 47 +++- .../reconcile/tests/stale_republish_tests.rs | 221 ++++++++++++++++++ 2 files changed, 267 insertions(+), 1 deletion(-) diff --git a/desktop/src-tauri/src/managed_agents/reconcile.rs b/desktop/src-tauri/src/managed_agents/reconcile.rs index 8a47aff6a..2ddd31119 100644 --- a/desktop/src-tauri/src/managed_agents/reconcile.rs +++ b/desktop/src-tauri/src/managed_agents/reconcile.rs @@ -115,7 +115,7 @@ fn reconcile_agents_in_dir_at( continue; } - if retain_agent_record(&conn, keys, record)? { + if retain_agent_record_at_boot(&conn, keys, record)? { reconciled += 1; } } @@ -123,6 +123,51 @@ fn reconcile_agents_in_dir_at( Ok(reconciled) } +/// Boot-only variant of [`retain_agent_record`]: reconciles the kind:30177 +/// identity record exactly as the interactive paths do, but publishes the +/// kind:30179 private config **only when no retained head exists**. +/// +/// Boot reads `managed-agents.json` raw — there is no overlay to resolve +/// against, because `hydrate_private_config_overlay` runs after this leg and +/// depends on the very rows written here. On a device that FOLLOWS another +/// device's config, disk is stale by construction (inbound 30179 updates the +/// overlay and retention, never the JSON), so rebuilding the 30179 projection +/// from disk republishes every stale field over a newer head as an audit-clean +/// gen+1 successor, and `monotonic_created_at` makes it win LWW. That fires at +/// launch, unprompted, and re-arms on every new head the follower receives. +/// +/// Restricting boot to the head-absent case keeps the requirement boot exists +/// to serve — an agent whose nsec lives in the keyring must get its FIRST +/// 30179 published — while leaving an existing head to the interactive edit +/// paths, which resolve the overlay before retaining and so author from +/// relay-fresh state. Every 30177 (no-secrets projection) behaves exactly as +/// before: the upgrade republish waves run on that kind, not this one. +fn retain_agent_record_at_boot( + conn: &rusqlite::Connection, + keys: &nostr::Keys, + record: &ManagedAgentRecord, +) -> Result { + let transaction = conn + .unchecked_transaction() + .map_err(|error| format!("failed to begin agent retention transaction: {error}"))?; + let public_changed = retain_public_agent_record(&transaction, keys, record)?; + let private_head = get_retained_event( + &transaction, + KIND_PRIVATE_MANAGED_AGENT, + &keys.public_key().to_hex(), + &record.pubkey, + )?; + let private_changed = if private_head.is_some() { + false + } else { + retain_private_agent_record(&transaction, keys, record)? + }; + transaction + .commit() + .map_err(|error| format!("failed to commit agent retention transaction: {error}"))?; + Ok(public_changed || private_changed) +} + /// 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 diff --git a/desktop/src-tauri/src/managed_agents/reconcile/tests/stale_republish_tests.rs b/desktop/src-tauri/src/managed_agents/reconcile/tests/stale_republish_tests.rs index a0e8499bd..6a895c84b 100644 --- a/desktop/src-tauri/src/managed_agents/reconcile/tests/stale_republish_tests.rs +++ b/desktop/src-tauri/src/managed_agents/reconcile/tests/stale_republish_tests.rs @@ -702,3 +702,224 @@ fn sami_fix_pair_start_resolve_then_snapshot_keeps_quad_definition_authoritative "control: quad still definition-authoritative" ); } + +// ── Review item 5: boot reconcile as a stale-republish site ───────────────── +// +// `reconcile_agents_in_dir_at` reads `managed-agents.json` raw and cannot +// resolve the overlay: `hydrate_private_config_overlay` runs AFTER this leg +// (`event_sync.rs:19-20`) and reads the rows this leg writes. On a following +// device, disk is stale by construction, so rebuilding the 30179 from disk is +// the item-2 clobber with no user action at all. + +/// Builds the follower fixture: a stale disk store plus a NEWER inbound 30179 +/// head (`pending_sync = 0`, far-future `created_at`). Returns the head event. +fn seed_follower_with_newer_head( + dir: &TempDir, + owner_keys: &nostr::Keys, + disk: &ManagedAgentRecord, + fresh: &ManagedAgentRecord, + generation: u64, + created_at: i64, +) -> nostr::Event { + let owner_hex = owner_keys.public_key().to_hex(); + let payload = + private_payload_from_record(fresh, &owner_hex, generation, Some("aa".repeat(32))).unwrap(); + let event = + private_managed_agent::build_event(owner_keys, &payload, created_at as u64).unwrap(); + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + crate::managed_agents::retention::retain_inbound_event( + &conn, + &RetainedEvent { + kind: KIND_PRIVATE_MANAGED_AGENT, + pubkey: owner_hex, + d_tag: disk.pubkey.clone(), + content: event.content.clone(), + created_at, + raw_event: event.as_json(), + pending_sync: false, + }, + ) + .unwrap(); + event +} + +fn retained_private_row(dir: &TempDir, owner_keys: &nostr::Keys, pubkey: &str) -> RetainedEvent { + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + get_retained_event( + &conn, + KIND_PRIVATE_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + pubkey, + ) + .unwrap() + .unwrap() +} + +/// Item 5 FIX: boot reconcile must leave an existing 30179 head alone. +/// +/// Red-first against `retain_agent_record` at boot: the probe measured +/// `name="stale-disk-name"`, `parallelism=Some(1)`, gen 5→6, `prev` = the +/// clobbered head, `created_at` = head+1 (so it wins LWW), `pending_sync=true` +/// — every stale disk field published over device A's newer config at launch, +/// with no user action. +#[test] +fn boot_reconcile_leaves_existing_private_head_intact() { + let dir = TempDir::new().unwrap(); + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + + let mut disk = sample_record(&pubkey, "stale-disk-name"); + disk.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + disk.system_prompt = Some("STALE disk prompt".into()); + disk.parallelism = 1; + disk.env_vars = BTreeMap::from([("STALE_KEY".to_string(), "stale".to_string())]); + write_store(&dir, &[disk.clone()]); + + let mut fresh = disk.clone(); + fresh.name = "FRESH relay name".into(); + fresh.system_prompt = Some("FRESH relay prompt".into()); + fresh.parallelism = 16; + fresh.env_vars = BTreeMap::from([("FRESH_KEY".to_string(), "fresh".to_string())]); + let head_created_at = nostr::Timestamp::now().as_secs() as i64 + 10_000; + let head_event = + seed_follower_with_newer_head(&dir, &owner_keys, &disk, &fresh, 5, head_created_at); + + // BOOT. No user action. + reconcile_agents_in_dir(dir.path(), &owner_keys).unwrap(); + + let row = retained_private_row(&dir, &owner_keys, &pubkey); + assert_eq!( + row.raw_event, + head_event.as_json(), + "boot reconcile must not rebuild the 30179 from stale disk over an \ + existing head — byte-identical, so no gen bump and no re-encryption" + ); + // Not merely equal-by-content: nothing was queued for publish either. + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + assert!( + get_pending_sync(&conn) + .unwrap() + .iter() + .all(|event| event.kind != KIND_PRIVATE_MANAGED_AGENT), + "no stale 30179 enqueued for relay publish" + ); +} + +/// The requirement item 1 exists to serve, preserved: an agent with NO retained +/// 30179 head still publishes its first one at boot. Without this arm the fix +/// above is satisfied by never publishing a 30179 at boot at all — which is the +/// item-1 bug restored. +#[test] +fn boot_reconcile_still_publishes_first_private_config() { + let dir = TempDir::new().unwrap(); + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + + let mut record = sample_record(&pubkey, "untouched-agent"); + record.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + record.parallelism = 7; + write_store(&dir, &[record]); + + assert_eq!(reconcile_agents_in_dir(dir.path(), &owner_keys).unwrap(), 1); + + let row = retained_private_row(&dir, &owner_keys, &pubkey); + let (_, payload) = private_managed_agent::validate_and_decrypt( + &nostr::Event::from_json(&row.raw_event).unwrap(), + &owner_keys, + ) + .unwrap(); + assert_eq!(payload.generation, 1); + assert_eq!(payload.previous_event_id, None); + assert_eq!(payload.config.parallelism, Some(7)); + assert!(row.pending_sync, "first 30179 is queued for publish"); +} + +/// The 30177 leg must be untouched by the 30179 gate: an edited record whose +/// PUBLIC projection changed still republishes at boot even though a private +/// head exists. This is what keeps the upgrade republish waves +/// (`slimming_republish_wave_is_one_time`) working, and it fails if the gate is +/// written at the wrong level (skipping the whole record instead of the 30179). +#[test] +fn boot_reconcile_still_republishes_public_projection_with_private_head_present() { + let dir = TempDir::new().unwrap(); + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + + let mut disk = sample_record(&pubkey, "public-name-v2"); + disk.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + write_store(&dir, &[disk.clone()]); + + let mut fresh = disk.clone(); + fresh.parallelism = 16; + let head_created_at = nostr::Timestamp::now().as_secs() as i64 + 10_000; + seed_follower_with_newer_head(&dir, &owner_keys, &disk, &fresh, 5, head_created_at); + + assert_eq!( + reconcile_agents_in_dir(dir.path(), &owner_keys).unwrap(), + 1, + "the 30177 identity projection still reconciles at boot" + ); + let conn = open_retention_db(&dir.path().join("retention.db")).unwrap(); + let public_row = get_retained_event( + &conn, + KIND_MANAGED_AGENT, + &owner_keys.public_key().to_hex(), + &pubkey, + ) + .unwrap() + .unwrap(); + assert!(public_row.content.contains("public-name-v2")); + assert!(public_row.pending_sync); +} + +/// WRONG-FIX PROBE (permanent): the tempting alternative is to resolve the +/// overlay inside boot reconcile (swapping the hydrate/reconcile order in +/// `run_event_sync`). It is wrong for the same reason the centralized resolve +/// was wrong at the edit site — but with a worse blast radius, because boot +/// touches EVERY agent rather than the one being edited. +/// +/// A local edit made while the relay was unreachable lives on disk AND in a +/// `pending_sync` 30179 that never flushed. Resolving disk through an overlay +/// hydrated from the last-known head would rebuild the payload from that older +/// head and discard the edit — at launch, silently, for every agent. +#[test] +fn probe_resolving_overlay_at_boot_would_discard_unflushed_local_edits() { + use crate::managed_agents::private_config_overlay::{PrivateConfigOverlay, PrivateConfigPatch}; + + let owner_keys = nostr::Keys::generate(); + let agent_keys = nostr::Keys::generate(); + let pubkey = agent_keys.public_key().to_hex(); + let owner_hex = owner_keys.public_key().to_hex(); + + // The last head this device saw, which is what a boot-time overlay would + // hydrate from. + let mut head = sample_record(&pubkey, "head-name"); + head.private_key_nsec = agent_keys.secret_key().to_bech32().unwrap(); + head.parallelism = 16; + let head_payload = private_payload_from_record(&head, &owner_hex, 3, None).unwrap(); + let mut overlay = PrivateConfigOverlay::default(); + overlay.insert_patch(PrivateConfigPatch::from_payload(head_payload).unwrap()); + + // The user's offline edit, on disk and not yet flushed to the relay. + let mut disk = head.clone(); + disk.parallelism = 2; + + assert_eq!( + overlay.resolve_local_record(&disk).parallelism, + 16, + "resolving at boot DISCARDS the unflushed local edit (2 -> 16); this is \ + why the fix is a head-presence gate, not a resolve" + ); + // Positive control: with no patch the edit survives, so the discard above + // is the overlay winning rather than a broken fixture. + assert_eq!( + PrivateConfigOverlay::default() + .resolve_local_record(&disk) + .parallelism, + 2, + "control: without an overlay patch the offline edit survives" + ); +}