jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-11-10 11:21:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: harden three mutation-escape test paths from Paul's bounce

Browse Source 
Three test-correctness fixes, no production logic changes:

1. test_retain_agent_record_user_intent_clears_gate_but_reconcile_does_not:
   The interactive-save call previously used the same record (unchanged
   content), so the early-return branch fired and set_publish_blocked(false)
   cleared the gate before the retain_user_intent_event path was reached.
   Reverting the changed-content branch to retain_event left the test green.
   Added Case B: interactive save with a different system_prompt, which
   exercises the retain_user_intent_event path directly. Also added
   Case D: reconcile with changed content, confirming retain_event does
   not clear the gate. Now all four combinations (same/changed × user/reconcile)
   are verified.

2. test_non_persona_tombstone_user_intent_clears_gate:
   Previously called retain_user_intent_event directly — reverting
   tombstone_managed_agent_pending to retain_event left zero test callers
   on the production path, so the suite stayed green. Extracted
   tombstone_agent_pending_inner from commands/agents.rs into
   managed_agents/reconcile.rs (pub(crate), symmetric with retain_agent_record)
   and rewired tombstone_managed_agent_pending to call it. Test now drives
   the seam directly. Pre-seeds the tombstone coordinate with
   publish_blocked=true so retain_event (which leaves the existing value)
   and retain_user_intent_event (which calls set_publish_blocked(false))
   produce distinguishable outcomes. Mutation at the production call site
   now fails the suite.

3. test_mark_synced_and_stamp_baseline_atomic_both_written_on_success:
   Previously tested the success path only. Added Case B: drops the
   persona_baselines table before the call, forces the baseline INSERT to
   fail, and asserts the call errors AND pending_sync is still 1
   (transaction rolled back). If the function is split back into two
   autocommit statements, the UPDATE commits first and pending_sync becomes 0
   before the INSERT fails — the rollback assertion catches the regression.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
2026-08-01 15:54:31 -04:00
parent 7e3823a802
commit e45f8dc9a2
3 changed files with 197 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
desktop/src-tauri/src/commands/agents.rs
View File

@@ -78,33 +78,15 @@ pub(super) fn tombstone_managed_agent_pending(
state: &AppState,
agent_pubkey: &str,
) {
use crate::managed_agents::{
agent_events::build_agent_delete,
retention::{
delete_retained_event, open_retention_db, retain_user_intent_event,
tombstone_retention_d_tag, RetainedEvent,
},
};
use buzz_core_pkg::kind::KIND_MANAGED_AGENT;
const KIND_DELETE: u32 = 5;
use crate::managed_agents::retention::open_retention_db;
let result = (|| -> Result<(), String> {
let scope = crate::managed_agents::retention::active_retention_scope(app, state)?;
let owner_pubkey = scope.owner_keys.public_key().to_hex();
let event = build_agent_delete(agent_pubkey, &owner_pubkey)?
.sign_with_keys(&scope.owner_keys)
.map_err(|e| format!("failed to sign managed-agent tombstone: {e}"))?;
let conn = open_retention_db(&scope.db_path)?;
delete_retained_event(&conn, KIND_MANAGED_AGENT, &owner_pubkey, agent_pubkey)?;
retain_user_intent_event(
crate::managed_agents::reconcile::tombstone_agent_pending_inner(
&conn,
&RetainedEvent::pending(
KIND_DELETE,
owner_pubkey,
tombstone_retention_d_tag(KIND_MANAGED_AGENT, agent_pubkey),
&event,
),
&scope.owner_keys,
agent_pubkey,
)
})();
if let Err(e) = result {

210
desktop/src-tauri/src/managed_agents/config_sync/regression_tests.rs
View File

@@ -394,13 +394,19 @@ fn test_user_edit_reopens_gated_row_but_reconcile_retain_does_not() {
// ── (c-ext) Thufir pass-1 finding #2: retain_agent_record user-intent clears
// the gate; boot-reconcile (user_intent=false) does NOT.
//
// Mutation-sensitive: passing `false` (reconcile mode) to both calls would
// leave the gate set after the first assert succeeds (reconcile doesn't clear
// it) and would cause the second assert to pass trivially — but the assert on
// the gate being CLEAR after user_intent=true would fail because nothing
// cleared it, catching the regression. Conversely, passing `true` to both
// would make the reconcile call incorrectly clear the gate, failing the final
// "reconcile must NOT clear" assertion.
// Two cases exercised:
//
// Case A — unchanged content (same record re-saved). The early-return branch
// in retain_agent_record fires; user-intent clears the gate via
// `set_publish_blocked(false)`. Mutation: removing the early-return
// `set_publish_blocked` call leaves the gate set → assertion fails.
//
// Case B — changed content (record with different system_prompt). The
// changed-content branch fires; user-intent calls `retain_user_intent_event`
// which clears publish_blocked. Mutation: reverting that branch to
// `retain_event` leaves the gate set → assertion fails. This is the critical
// path Paul's mutation revealed: a user who actually EDITS a parked agent must
// also see the gate cleared.
#[test]
fn test_retain_agent_record_user_intent_clears_gate_but_reconcile_does_not() {
let conn = test_db();
@@ -408,53 +414,97 @@ fn test_retain_agent_record_user_intent_clears_gate_but_reconcile_does_not() {
let owner = keys.public_key().to_hex();
let record = make_sample_agent_record();
// ── Case A: unchanged content ──────────────────────────────────────────
// Seed the row as blocked (simulates what the boot barrier would have set).
retain_agent_record(&conn, &keys, &record, false).unwrap();
set_publish_blocked(&conn, KIND_MANAGED_AGENT, &owner, &record.pubkey, true).unwrap();
assert!(
is_publish_blocked(&conn, KIND_MANAGED_AGENT, &owner, &record.pubkey).unwrap(),
"precondition: gate must be set"
"precondition: gate must be set (case A)"
);
// Interactive save (user_intent = true) must reopen the gate.
// Interactive save with SAME content — early-return branch, still clears gate.
retain_agent_record(&conn, &keys, &record, true).unwrap();
assert!(
!is_publish_blocked(&conn, KIND_MANAGED_AGENT, &owner, &record.pubkey).unwrap(),
"user-intent retain_agent_record must clear publish_blocked"
"user-intent retain_agent_record must clear publish_blocked even for unchanged content (case A)"
);
// Re-gate to simulate a fresh boot decision.
// Re-gate for case B.
set_publish_blocked(&conn, KIND_MANAGED_AGENT, &owner, &record.pubkey, true).unwrap();
// Boot reconcile (user_intent = false) must NOT clear it.
// ── Case B: changed content ────────────────────────────────────────────
// Mutate the record's system prompt so the changed-content branch fires,
// exercising the retain_user_intent_event path (not just set_publish_blocked).
let mut edited_record = make_sample_agent_record();
edited_record.system_prompt =
Some("EDITED PROMPT — triggers the changed-content branch".to_string());
retain_agent_record(&conn, &keys, &edited_record, true).unwrap();
assert!(
!is_publish_blocked(&conn, KIND_MANAGED_AGENT, &owner, &edited_record.pubkey).unwrap(),
"user-intent retain_agent_record must clear publish_blocked for changed content (case B)"
);
// Re-gate to test the reconcile path (changed content, user_intent=false).
set_publish_blocked(&conn, KIND_MANAGED_AGENT, &owner, &record.pubkey, true).unwrap();
// Boot reconcile (user_intent = false) must NOT clear it — either branch.
retain_agent_record(&conn, &keys, &record, false).unwrap();
assert!(
is_publish_blocked(&conn, KIND_MANAGED_AGENT, &owner, &record.pubkey).unwrap(),
"reconcile retain_agent_record must NOT clear publish_blocked"
"reconcile retain_agent_record must NOT clear publish_blocked (same-content, case C)"
);
set_publish_blocked(&conn, KIND_MANAGED_AGENT, &owner, &record.pubkey, true).unwrap();
let mut reconcile_edit = make_sample_agent_record();
reconcile_edit.system_prompt =
Some("RECONCILE EDIT — changed-content branch, reconcile mode".to_string());
retain_agent_record(&conn, &keys, &reconcile_edit, false).unwrap();
assert!(
is_publish_blocked(&conn, KIND_MANAGED_AGENT, &owner, &record.pubkey).unwrap(),
"reconcile retain_agent_record must NOT clear publish_blocked (changed-content, case D)"
);
}
// ── (c-ext2) Thufir pass-1 finding #2: non-persona tombstone (kind:5 for
// a managed-agent coordinate) via retain_user_intent_event clears the gate;
// bare retain_event does NOT.
// ── (c-ext2) Thufir pass-1 finding #2: agent tombstone via the production seam
// (`tombstone_agent_pending_inner`) clears the gate; the pre-existing row gate
// confirms that a plain `retain_event` call would NOT clear it.
//
// Mutation-sensitive: the production paths in `tombstone_managed_agent_pending`
// and `tombstone_team_pending` now call `retain_user_intent_event`; if they
// were accidentally reverted to `retain_event`, the first assertion would fail.
// Mutation-sensitive: `tombstone_agent_pending_inner` calls
// `retain_user_intent_event`. `retain_event` alone does NOT update
// `publish_blocked` in its ON CONFLICT SET clause — it leaves the existing
// value unchanged, so a pre-seeded gate survives. `retain_user_intent_event`
// always calls `set_publish_blocked(false)` after the upsert. If
// `tombstone_agent_pending_inner` is reverted to `retain_event`, the pre-seeded
// gate stays set and the post-tombstone assertion fails.
//
// The test drives the production seam directly, so a missing or changed
// production caller is caught without depending on AppHandle.
#[test]
fn test_non_persona_tombstone_user_intent_clears_gate() {
use nostr::JsonUtil;
use crate::managed_agents::reconcile::tombstone_agent_pending_inner;
let conn = test_db();
const KIND_DELETE: u32 = 5;
let tombstone_d = tombstone_retention_d_tag(KIND_MANAGED_AGENT, "agent-abc123");
let keys = nostr::Keys::generate();
let owner = keys.public_key().to_hex();
let event = nostr::EventBuilder::new(nostr::Kind::Custom(KIND_DELETE as u16), "{}".to_string())
.sign_with_keys(&keys)
.unwrap();
// The agent coordinate to be tombstoned.
let agent_pubkey = "abababababababababababababababababababababababababababababababababab";
// Gate a placeholder row at the tombstone coordinate.
// The tombstone coordinate is (KIND_DELETE=5, owner, tombstone_d_tag).
use crate::managed_agents::retention::tombstone_retention_d_tag;
const KIND_DELETE: u32 = 5;
let tombstone_d = tombstone_retention_d_tag(KIND_MANAGED_AGENT, agent_pubkey);
// Pre-seed the tombstone coordinate as blocked. This simulates the case
// where the coordinate was previously parked by the boot barrier.
// retain_event does NOT clear publish_blocked on conflict — only
// retain_user_intent_event does. Seeding here forces the test to distinguish
// the two paths on the pre-existing row, making it mutation-sensitive.
let placeholder_event =
nostr::EventBuilder::new(nostr::Kind::Custom(KIND_DELETE as u16), "{}".to_string())
.sign_with_keys(&keys)
.unwrap();
retain_event(
&conn,
&RetainedEvent {
@@ -462,9 +512,12 @@ fn test_non_persona_tombstone_user_intent_clears_gate() {
pubkey: owner.clone(),
d_tag: tombstone_d.clone(),
content: "{}".to_string(),
created_at: event.created_at.as_secs() as i64,
raw_event: event.as_json(),
event_id: Some(event.id.to_hex()),
created_at: placeholder_event.created_at.as_secs() as i64,
raw_event: {
use nostr::JsonUtil;
placeholder_event.as_json()
},
event_id: Some(placeholder_event.id.to_hex()),
pending_sync: false,
publish_blocked: false,
},
@@ -473,43 +526,47 @@ fn test_non_persona_tombstone_user_intent_clears_gate() {
set_publish_blocked(&conn, KIND_DELETE, &owner, &tombstone_d, true).unwrap();
assert!(
is_publish_blocked(&conn, KIND_DELETE, &owner, &tombstone_d).unwrap(),
"precondition: gate must be set"
"precondition: tombstone coordinate must be gated before the seam call"
);
// User-intent tombstone retain must clear the gate.
let event2 =
nostr::EventBuilder::new(nostr::Kind::Custom(KIND_DELETE as u16), "{}".to_string())
.sign_with_keys(&keys)
.unwrap();
retain_user_intent_event(
&conn,
&RetainedEvent {
kind: KIND_DELETE,
pubkey: owner.clone(),
d_tag: tombstone_d.clone(),
content: "{}".to_string(),
created_at: event2.created_at.as_secs() as i64 + 1,
raw_event: event2.as_json(),
event_id: Some(event2.id.to_hex()),
pending_sync: true,
publish_blocked: false,
},
)
.unwrap();
// Also seed the agent row (tombstone_agent_pending_inner deletes it).
retain_agent_record(&conn, &keys, &make_sample_agent_record(), false).unwrap();
// Run the production tombstone seam through the conn+keys boundary.
// This must: (1) delete the agent's pending row, and (2) retain a user-intent
// kind:5 tombstone whose post-upsert set_publish_blocked call clears the gate.
tombstone_agent_pending_inner(&conn, &keys, agent_pubkey).unwrap();
// Gate must NOT be set on the tombstone row — user intent cleared it.
assert!(
!is_publish_blocked(&conn, KIND_DELETE, &owner, &tombstone_d).unwrap(),
"user-intent tombstone retain must clear publish_blocked"
"tombstone_agent_pending_inner must clear publish_blocked on the tombstone coordinate"
);
// The tombstone row must be pending for relay sync.
let tombstone_row = get_retained_event(&conn, KIND_DELETE, &owner, &tombstone_d)
.unwrap()
.expect("tombstone row must exist after tombstone_agent_pending_inner");
assert!(
tombstone_row.pending_sync,
"tombstone row must have pending_sync = 1 (queued for relay)"
);
}
// ── (f) Thufir pass-1 finding: mark_synced_and_stamp_baseline atomically
// clears pending AND stamps the baseline in a single transaction.
//
// Mutation-sensitive: if the function is split back into two independent
// autocommit statements, the test still passes on the happy path — but the
// invariant that "pending_sync = 0 iff baseline is stamped" becomes testable
// by verifying both columns together. If only one statement ran, one of the
// two final assertions would fail, catching the regression.
// Two cases:
//
// Case A (success): both effects land together. Mutation: splitting back into
// two autocommit statements still passes the happy path — but see case B.
//
// Case B (failure): the persona_baselines table is dropped before the call,
// forcing the baseline INSERT to fail. The transaction must roll back — meaning
// pending_sync must remain 1. Mutation: if the helper is split back into two
// autocommit statements (mark_synced first, then set_baseline_full), the first
// statement clears pending_sync BEFORE the baseline INSERT fails; pending_sync
// then reads 0, failing the rollback assertion and catching the regression.
#[test]
fn test_mark_synced_and_stamp_baseline_atomic_both_written_on_success() {
let conn = test_db();
@@ -547,6 +604,7 @@ fn test_mark_synced_and_stamp_baseline_atomic_both_written_on_success() {
"no baseline before mark_synced_and_stamp_baseline"
);
// ── Case A: success path ──────────────────────────────────────────────
mark_synced_and_stamp_baseline(
&conn,
KIND_PERSONA,
@@ -579,6 +637,46 @@ fn test_mark_synced_and_stamp_baseline_atomic_both_written_on_success() {
baseline.1, content,
"baseline content must match the published content"
);
// ── Case B: failure path — transaction must roll back ─────────────────
// Re-set pending_sync so there is something to clear.
conn.execute(
"UPDATE persona_events SET pending_sync = 1 WHERE kind = ?1 AND pubkey = ?2 AND d_tag = ?3",
rusqlite::params![KIND_PERSONA, OWNER, D_TAG],
)
.unwrap();
assert!(
get_retained_event(&conn, KIND_PERSONA, OWNER, D_TAG)
.unwrap()
.unwrap()
.pending_sync,
"precondition: pending_sync must be 1 before failure test"
);
// Drop persona_baselines so the baseline INSERT inside the transaction fails.
conn.execute_batch("DROP TABLE persona_baselines").unwrap();
// The call must return an error.
let result = mark_synced_and_stamp_baseline(
&conn,
KIND_PERSONA,
OWNER,
D_TAG,
created_at,
&content,
&event_id,
false,
);
assert!(result.is_err(), "must error when baseline table is absent");
// The transaction must have rolled back — pending_sync must still be 1.
assert!(
get_retained_event(&conn, KIND_PERSONA, OWNER, D_TAG)
.unwrap()
.unwrap()
.pending_sync,
"pending_sync must remain 1 when the transaction rolled back (atomicity contract)"
);
}
/// Helper shared by the agent-record tests in this module.

39
desktop/src-tauri/src/managed_agents/reconcile.rs
View File

@@ -166,5 +166,44 @@ pub(crate) fn reconcile_agents_in_dir(base_dir: &Path, keys: &nostr::Keys) -> Re
reconcile_agents_in_dir_at(base_dir, keys, &base_dir.join("retention.db"))
}
/// AppHandle-free core of the agent tombstone flow. Extracted as a seam so the
/// retention logic is independently testable without a running Tauri app.
///
/// Deletes the pending agent row at `(30177, owner, agent_pubkey)` and retains
/// a user-intent kind:5 tombstone at `(5, owner, tombstone_d_tag)`. The
/// user-intent write unconditionally clears `publish_blocked` so a tombstone
/// always propagates in-session, even if the coordinate was previously parked.
pub(crate) fn tombstone_agent_pending_inner(
conn: &rusqlite::Connection,
keys: &nostr::Keys,
agent_pubkey: &str,
) -> Result<(), String> {
use super::{
agent_events::build_agent_delete,
retention::{
delete_retained_event, retain_user_intent_event, tombstone_retention_d_tag,
RetainedEvent,
},
};
use buzz_core_pkg::kind::KIND_MANAGED_AGENT;
const KIND_DELETE: u32 = 5;
let owner_pubkey = keys.public_key().to_hex();
let event = build_agent_delete(agent_pubkey, &owner_pubkey)?
.sign_with_keys(keys)
.map_err(|e| format!("failed to sign managed-agent tombstone: {e}"))?;
delete_retained_event(conn, KIND_MANAGED_AGENT, &owner_pubkey, agent_pubkey)?;
retain_user_intent_event(
conn,
&RetainedEvent::pending(
KIND_DELETE,
owner_pubkey,
tombstone_retention_d_tag(KIND_MANAGED_AGENT, agent_pubkey),
&event,
),
)
}
#[cfg(test)]
mod tests;