mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): close round-2 residuals in workspace-scoped agent library
F1: canonical identity indexing — binding validation parses every owner and agent pubkey through a canonical-encoding gate (64 lowercase hex + curve-point check, mirroring parse_canonical_pubkey), rejecting any non-canonical spelling into the quarantine ladder. This makes the document-wide raw-string identity index sound: a cross-owner alias can no longer evade the check by re-casing its hex. F3: command-boundary preflight — raw_record_by_slug and MutationRoute gain live consumers. delete_persona and snapshot/team import route on for_slug; both inbound arms route on for_persona_d_tag (their real match key, derived from source_team_persona_slug). Each consults the RAW keyless store BEFORE any destructive effect, so a library-projected target fails closed with zero side effects until §3 wires the state machine. The merge seam now compares a SharedSlotFingerprint over the SharedDefinition allowlist instead of the whole record, so scope-local edits (is_active, env_vars, timestamps) on a projected record ride the plain path. F4: the legacy byte-compat test drives the real store save->load->save path and asserts a byte-exact fixpoint against the pinned-head baseline, plus key-absence — the assertion now matches its name. F5: deferred_archives is private; upsert_deferred_archive and deferred_archive_obligations are the only access outside the module. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
@@ -7,9 +7,9 @@ use tauri::{AppHandle, Emitter, Manager};
|
||||
use crate::{
|
||||
app_state::AppState,
|
||||
managed_agents::{
|
||||
agent_events::ManagedAgentEventContent, load_personas, persona_events::persona_d_tag,
|
||||
save_personas, team_events::TeamEventContent, try_regenerate_nest, AgentDefinition,
|
||||
ManagedAgentRecord, TeamRecord,
|
||||
agent_events::ManagedAgentEventContent, load_agent_definitions, load_personas,
|
||||
persona_events::persona_d_tag, save_personas, team_events::TeamEventContent,
|
||||
try_regenerate_nest, AgentDefinition, ManagedAgentRecord, MutationRoute, TeamRecord,
|
||||
},
|
||||
util::now_iso,
|
||||
};
|
||||
@@ -134,6 +134,25 @@ fn reconcile_inbound_persona_event_blocking(
|
||||
else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
// Library-projection preflight (§2.7): a projected persona is
|
||||
// library-authoritative, so an inbound plain-persona upsert targeting it must
|
||||
// NOT overwrite the local cache OR advance the retention head — the future
|
||||
// library-aware inbound handler owns that coordinate. Return WITHOUT retaining
|
||||
// (Ok, unretained), so once that handler lands the event is reprocessed;
|
||||
// retaining here would move the head and make the retry look stale. Routes on
|
||||
// `persona_d_tag` (the exact match key `apply_inbound_persona` uses) against
|
||||
// the RAW keyless record — `library_ref` is not view-carried. Only KIND_PERSONA
|
||||
// targets the persona store; team/agent projections are out of scope here.
|
||||
if kind == KIND_PERSONA {
|
||||
let raw_definitions = load_agent_definitions(&app)?;
|
||||
if MutationRoute::for_persona_d_tag(&raw_definitions, &d_tag)
|
||||
== MutationRoute::LibraryProjected
|
||||
{
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
|
||||
let conn = open_retention_db(&scope.db_path)?;
|
||||
let outcome = retain_inbound_event(
|
||||
&conn,
|
||||
@@ -278,6 +297,25 @@ fn reconcile_inbound_tombstone(
|
||||
else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
// Library-projection preflight (§2.7): a projected persona is
|
||||
// library-authoritative, so an inbound tombstone targeting it must NOT delete
|
||||
// the local record OR advance the retention head — the future library-aware
|
||||
// handler owns removing that coordinate (as a §3.4 workspace-remove). Return
|
||||
// WITHOUT retaining (Ok, unretained) so the tombstone is reprocessed once that
|
||||
// handler lands. Routes on the tombstone's `target_d_tag` — the same
|
||||
// `persona_d_tag`-derived key the KIND_PERSONA `retain` below matches on —
|
||||
// against the RAW keyless record. Only KIND_PERSONA tombstones touch the
|
||||
// persona store; team/agent removals are out of scope here.
|
||||
if target_kind == KIND_PERSONA {
|
||||
let raw_definitions = load_agent_definitions(app)?;
|
||||
if MutationRoute::for_persona_d_tag(&raw_definitions, &target_d_tag)
|
||||
== MutationRoute::LibraryProjected
|
||||
{
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
|
||||
let conn = open_retention_db(&scope.db_path)?;
|
||||
let outcome = retain_inbound_event(
|
||||
&conn,
|
||||
|
||||
@@ -3,10 +3,11 @@ use tauri::AppHandle;
|
||||
use crate::{
|
||||
app_state::AppState,
|
||||
managed_agents::{
|
||||
current_instance_id, delete_agent_key, load_managed_agents, load_persona_views,
|
||||
load_personas, load_teams, save_managed_agents, save_personas, stop_managed_agent_process,
|
||||
sync_managed_agent_processes, try_regenerate_nest, validate_persona_activation_change,
|
||||
validate_persona_deletion, AgentDefinition, ManagedAgentRecord, PersonaView,
|
||||
current_instance_id, delete_agent_key, load_agent_definitions, load_managed_agents,
|
||||
load_persona_views, load_personas, load_teams, save_managed_agents, save_personas,
|
||||
stop_managed_agent_process, sync_managed_agent_processes, try_regenerate_nest,
|
||||
validate_persona_activation_change, validate_persona_deletion, AgentDefinition,
|
||||
ManagedAgentRecord, MutationRoute, PersonaView,
|
||||
},
|
||||
util::now_iso,
|
||||
};
|
||||
@@ -155,6 +156,18 @@ pub async fn delete_persona(id: String, app: AppHandle) -> Result<(), String> {
|
||||
// so every deleted persona here is one this owner published.
|
||||
let d_tag = crate::managed_agents::persona_events::persona_d_tag(persona);
|
||||
|
||||
// Library-projection preflight (§2.7): a projected persona's delete
|
||||
// is a §3.4 workspace-remove that must journal an `ExcludePending`
|
||||
// intent, NOT a local cascade that destroys the linked instances. The
|
||||
// route is read from the RAW keyless record (`library_ref` is not
|
||||
// view-carried), under the store lock, BEFORE any runtime stop,
|
||||
// `commit_cascade_agents`, keyring deletion, or tombstone — so a
|
||||
// projected target reaches none of them. The save-seam guard would
|
||||
// also reject the persona save, but only after the cascade already
|
||||
// ran; this refusal is what keeps the delete atomic.
|
||||
let raw_definitions = load_agent_definitions(&app)?;
|
||||
MutationRoute::reject_projected_slug(&raw_definitions, &id)?;
|
||||
|
||||
// ── Phase 1: Stage ─────────────────────────────────────────────
|
||||
//
|
||||
// Load agents, sync process state, and build the cascade set. Lock
|
||||
|
||||
@@ -565,6 +565,17 @@ where
|
||||
let now = now_iso();
|
||||
let persona_id = uuid::Uuid::new_v4().to_string();
|
||||
|
||||
// Library-projection guard rail (§2.7): the import mints a fresh UUID
|
||||
// slug (`persona_id` above), so it can never target an existing projected
|
||||
// record — but the route is consulted before any store write to keep the
|
||||
// §2.7 invariant uniform across every command boundary. If a future
|
||||
// change ever reused a slug, this refuses before `save_personas_at` and
|
||||
// before any key is persisted. Reads the RAW keyless definitions (where
|
||||
// `library_ref` lives — it is not view-carried), loaded under this lock.
|
||||
let raw_definitions =
|
||||
crate::managed_agents::storage::load_agent_definitions_at(&definitions_dir)?;
|
||||
crate::managed_agents::MutationRoute::reject_projected_slug(&raw_definitions, &persona_id)?;
|
||||
|
||||
let persona = AgentDefinition {
|
||||
id: persona_id.clone(),
|
||||
display_name: display_name.clone(),
|
||||
|
||||
@@ -24,10 +24,10 @@ use crate::{
|
||||
},
|
||||
managed_agents::{
|
||||
agent_snapshot::{build_snapshot, AgentSnapshot, AgentSnapshotMemoryEntry, MemoryLevel},
|
||||
load_managed_agents, load_managed_agents_at, load_personas, load_personas_at, load_teams,
|
||||
load_teams_readonly, managed_agents_store_path_at, save_managed_agents_at,
|
||||
save_personas_at, save_teams_at, teams_store_path_at, AgentDefinition, ManagedAgentRecord,
|
||||
TeamRecord,
|
||||
load_agent_definitions_at, load_managed_agents, load_managed_agents_at, load_personas,
|
||||
load_personas_at, load_teams, load_teams_readonly, managed_agents_store_path_at,
|
||||
save_managed_agents_at, save_personas_at, save_teams_at, teams_store_path_at,
|
||||
AgentDefinition, ManagedAgentRecord, MutationRoute, TeamRecord,
|
||||
},
|
||||
relay::{effective_agent_relay_url, sync_managed_agent_profile},
|
||||
util::now_iso,
|
||||
@@ -683,6 +683,9 @@ where
|
||||
)?;
|
||||
|
||||
let existing_records = load_managed_agents_at(&definitions_dir)?;
|
||||
// §2.7 projection guard rail, consulted in the pre-write pubkey-collision
|
||||
// loop (unreachable today — imports mint fresh UUIDs — but uniform).
|
||||
let raw_definitions = load_agent_definitions_at(&definitions_dir)?;
|
||||
for m in &minted {
|
||||
if existing_records.iter().any(|r| r.pubkey == m.pubkey) {
|
||||
return Err(format!(
|
||||
@@ -690,6 +693,7 @@ where
|
||||
m.pubkey
|
||||
));
|
||||
}
|
||||
MutationRoute::reject_projected_slug(&raw_definitions, &m.definition.id)?;
|
||||
}
|
||||
|
||||
let agents_store_path = managed_agents_store_path_at(&definitions_dir);
|
||||
|
||||
@@ -156,13 +156,17 @@ pub(crate) struct LibraryEntry {
|
||||
/// PERMANENT journaled retirement markers in v1 (P17-C1): no v1 path
|
||||
/// discharges them; their presence keeps `key_archive_protected(pubkey)`
|
||||
/// true. SET semantics on `(library_id, scope_id, agent_pubkey)`
|
||||
/// (`library_id` is fixed per entry). Insert ONLY through
|
||||
/// [`upsert_deferred_archive`](LibraryEntry::upsert_deferred_archive) — every
|
||||
/// append is an upsert (P15-MINOR) — and read through
|
||||
/// [`deferred_archive_obligations`](LibraryEntry::deferred_archive_obligations),
|
||||
/// which collapses legacy duplicate rows to one obligation.
|
||||
/// (`library_id` is fixed per entry). PRIVATE by construction — the ONLY
|
||||
/// access outside this module is [`upsert_deferred_archive`] (every append
|
||||
/// an upsert, P15-MINOR) and [`deferred_archive_obligations`] (reads legacy
|
||||
/// duplicate rows as one obligation), so no crate caller can `.push()` a
|
||||
/// duplicate or observe multiplicity. Serde and the child-module tests reach
|
||||
/// the field directly by module ancestry, not visibility.
|
||||
///
|
||||
/// [`upsert_deferred_archive`]: LibraryEntry::upsert_deferred_archive
|
||||
/// [`deferred_archive_obligations`]: LibraryEntry::deferred_archive_obligations
|
||||
#[serde(default)]
|
||||
pub deferred_archives: Vec<DeferredArchive>,
|
||||
deferred_archives: Vec<DeferredArchive>,
|
||||
/// Authoritative per-scope membership — sole source for delete-confirm
|
||||
/// enumeration, tombstone completion, and key lifetime.
|
||||
pub projections: BTreeMap<String, ProjectionEntry>,
|
||||
@@ -453,21 +457,47 @@ fn classify_entries(document: LibraryDocument) -> LoadedLibrary {
|
||||
}
|
||||
}
|
||||
|
||||
/// A binding pubkey must be its own canonical `to_hex()` encoding: exactly 64
|
||||
/// lowercase hex chars that parse as a valid curve point (§2.5). Production
|
||||
/// writers only ever emit `to_hex()` output, so a non-canonical spelling is
|
||||
/// hand-edited data and fails closed into the quarantine ladder. This is what
|
||||
/// makes the RAW-string identity indexes in [`identity_collisions`] sound: one
|
||||
/// agent key cannot survive under two different spellings, so a cross-owner
|
||||
/// alias can never evade the document-wide check by re-casing its hex. Mirrors
|
||||
/// the tree's `agent_snapshot_envelope::parse_canonical_pubkey` rule — including
|
||||
/// its explicit `xonly()` curve validation, since nostr's `PublicKey::from_hex`
|
||||
/// only decodes 32 bytes and defers lift-x, so a non-point like `"f" * 64` would
|
||||
/// otherwise pass structurally — kept library-local so the error text is
|
||||
/// domain-appropriate.
|
||||
fn parse_canonical_binding_pubkey(field: &str, value: &str) -> Result<PublicKey, String> {
|
||||
if value.len() != 64
|
||||
|| !value
|
||||
.chars()
|
||||
.all(|c| c.is_ascii_digit() || ('a'..='f').contains(&c))
|
||||
{
|
||||
return Err(format!(
|
||||
"binding {field} {value} is not canonical (expected 64 lowercase hex chars)"
|
||||
));
|
||||
}
|
||||
let pubkey = PublicKey::from_hex(value)
|
||||
.map_err(|_| format!("binding {field} {value} is not a valid pubkey"))?;
|
||||
pubkey
|
||||
.xonly()
|
||||
.map_err(|_| format!("binding {field} {value} is not a curve point"))?;
|
||||
Ok(pubkey)
|
||||
}
|
||||
|
||||
/// Semantic binding validation (§2.5 read validation): every binding's auth tag
|
||||
/// must verify against its `agent_pubkey` AND embed exactly the owner pubkey it
|
||||
/// is keyed under. Also rejects an entry that binds one `agent_pubkey` under two
|
||||
/// different owners. Malformed pubkeys/tags fail closed.
|
||||
/// different owners. Non-canonical or malformed pubkeys and bad tags fail closed;
|
||||
/// canonical encoding is required so the document-wide identity index can key on
|
||||
/// raw strings safely (see [`parse_canonical_binding_pubkey`]).
|
||||
fn validate_entry_bindings(entry: &LibraryEntry) -> Result<(), String> {
|
||||
let mut seen_agents: HashMap<String, String> = HashMap::new();
|
||||
for (owner_hex, binding) in &entry.identity_bindings {
|
||||
let owner = PublicKey::from_hex(owner_hex)
|
||||
.map_err(|_| format!("binding owner {owner_hex} is not a valid pubkey"))?;
|
||||
let agent = PublicKey::from_hex(&binding.agent_pubkey).map_err(|_| {
|
||||
format!(
|
||||
"binding agent {} is not a valid pubkey",
|
||||
binding.agent_pubkey
|
||||
)
|
||||
})?;
|
||||
let owner = parse_canonical_binding_pubkey("owner", owner_hex)?;
|
||||
let agent = parse_canonical_binding_pubkey("agent", &binding.agent_pubkey)?;
|
||||
let embedded =
|
||||
buzz_sdk_pkg::nip_oa::verify_auth_tag(&binding.auth_tag, &agent).map_err(|e| {
|
||||
format!(
|
||||
|
||||
@@ -382,6 +382,87 @@ fn test_same_owner_agent_reuse_across_entries_stays_healthy() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_noncanonical_agent_spelling_cannot_alias_across_owners() {
|
||||
let dir = tempdir().unwrap();
|
||||
let owner_a = Keys::generate();
|
||||
let owner_b = Keys::generate();
|
||||
let agent = Keys::generate();
|
||||
// Entry A binds agent P (canonical lowercase) under owner A. Entry B binds
|
||||
// the SAME P but spelled UPPERCASE under owner B, with a still-VALID auth
|
||||
// tag (computed over the canonical key). Before the fix, the raw-string
|
||||
// index saw two distinct agent keys and quarantined neither — the §2.5
|
||||
// cross-owner process-global alias. Now B fails canonical validation at
|
||||
// pass 1 (the only rejection cause is the encoding), so the alias can never
|
||||
// reach the healthy set; A stays usable.
|
||||
let a = value_of(&entry(
|
||||
"lib-canon-a",
|
||||
"scope-a",
|
||||
"aria",
|
||||
Some((&owner_a, &agent)),
|
||||
));
|
||||
let mut e_b = entry("lib-canon-b", "scope-b", "bram", Some((&owner_b, &agent)));
|
||||
for binding in e_b.identity_bindings.values_mut() {
|
||||
binding.agent_pubkey = binding.agent_pubkey.to_uppercase();
|
||||
}
|
||||
let b = value_of(&e_b);
|
||||
write_doc(dir.path(), &doc(vec![a.clone(), b.clone()]));
|
||||
|
||||
match load_library(dir.path()) {
|
||||
LibraryLoad::Loaded(loaded) => {
|
||||
assert_eq!(loaded.healthy.len(), 1);
|
||||
assert_eq!(loaded.healthy[0].library_id, "lib-canon-a");
|
||||
assert!(loaded.quarantined.contains(&b));
|
||||
assert!(loaded.degradations.iter().any(|d| d.contains("canonical")));
|
||||
}
|
||||
other => panic!("expected Loaded, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_same_owner_noncanonical_spelling_is_not_a_false_cross_owner_collision() {
|
||||
let dir = tempdir().unwrap();
|
||||
let owner = Keys::generate();
|
||||
let agent = Keys::generate();
|
||||
// ONE owner, spelled canonically in entry A and UPPERCASE (re-keyed) in
|
||||
// entry B. A raw-string index would read the two owner spellings as two
|
||||
// different owners and falsely group-quarantine the healthy same-owner
|
||||
// reuse. Under canonical enforcement B is rejected for its encoding and A
|
||||
// survives — the valid reuse is NOT dragged down by a phantom collision.
|
||||
let a = value_of(&entry(
|
||||
"lib-owner-canon-a",
|
||||
"scope-a",
|
||||
"aria",
|
||||
Some((&owner, &agent)),
|
||||
));
|
||||
let mut e_b = entry(
|
||||
"lib-owner-canon-b",
|
||||
"scope-b",
|
||||
"bram",
|
||||
Some((&owner, &agent)),
|
||||
);
|
||||
let (owner_hex, binding) = e_b
|
||||
.identity_bindings
|
||||
.iter()
|
||||
.next()
|
||||
.map(|(k, v)| (k.clone(), v.clone()))
|
||||
.expect("one binding");
|
||||
e_b.identity_bindings.clear();
|
||||
e_b.identity_bindings
|
||||
.insert(owner_hex.to_uppercase(), binding);
|
||||
let b = value_of(&e_b);
|
||||
write_doc(dir.path(), &doc(vec![a, b.clone()]));
|
||||
|
||||
match load_library(dir.path()) {
|
||||
LibraryLoad::Loaded(loaded) => {
|
||||
assert_eq!(loaded.healthy.len(), 1);
|
||||
assert_eq!(loaded.healthy[0].library_id, "lib-owner-canon-a");
|
||||
assert!(loaded.quarantined.contains(&b));
|
||||
}
|
||||
other => panic!("expected Loaded, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_duplicate_library_id_group_quarantines_all_colliders() {
|
||||
let dir = tempdir().unwrap();
|
||||
|
||||
@@ -498,10 +498,10 @@ pub(crate) fn save_personas_at(
|
||||
|
||||
/// The canonical raw-record-by-slug lookup (§2.7, §8 Phase 0). One place
|
||||
/// resolves a slug to its authoritative on-disk record so the library-aware
|
||||
/// routing decision below is identical for delete, inbound upsert/tombstone,
|
||||
/// and snapshot/team import. Consumed by §3's removal and inbound branches once
|
||||
/// the projection state machine lands; used by [`MutationRoute::for_slug`] now.
|
||||
#[allow(dead_code)]
|
||||
/// routing decision is identical for delete, inbound upsert/tombstone, and
|
||||
/// snapshot/team import. Command preflights consult it BEFORE any destructive
|
||||
/// effect (`library_ref` is not view-carried, so the route can only be read
|
||||
/// from the raw store, never from an inbound/import view).
|
||||
pub(crate) fn raw_record_by_slug<'a>(
|
||||
records: &'a [ManagedAgentRecord],
|
||||
slug: &str,
|
||||
@@ -539,11 +539,85 @@ impl MutationRoute {
|
||||
}
|
||||
|
||||
/// The route for a mutation targeting `slug` against the raw store — the
|
||||
/// decision delete/inbound/import consult before taking the plain path.
|
||||
#[allow(dead_code)]
|
||||
/// decision delete and snapshot/team import consult before taking the plain
|
||||
/// path. The slug is the persona `id`, which is the raw record's `slug`.
|
||||
pub(crate) fn for_slug(records: &[ManagedAgentRecord], slug: &str) -> Self {
|
||||
Self::for_record(raw_record_by_slug(records, slug))
|
||||
}
|
||||
|
||||
/// The route for an inbound mutation identified by its persona d-tag. The
|
||||
/// inbound apply/tombstone arms match a local record by
|
||||
/// [`persona_d_tag`](crate::managed_agents::persona_events::persona_d_tag),
|
||||
/// NOT by slug — an in-app persona's d-tag is its `id`, but a team-sourced
|
||||
/// persona keys on `source_team_persona_slug`. Routing must consult the raw
|
||||
/// record under the SAME derivation the apply/tombstone uses, or a projected
|
||||
/// team persona would slip past a slug-only check. `library_ref` is not
|
||||
/// view-carried, so the route can only be read from the raw store.
|
||||
pub(crate) fn for_persona_d_tag(records: &[ManagedAgentRecord], d_tag: &str) -> Self {
|
||||
Self::for_record(records.iter().find(|record| {
|
||||
record.to_definition_view().is_some_and(|view| {
|
||||
crate::managed_agents::persona_events::persona_d_tag(&view) == d_tag
|
||||
})
|
||||
}))
|
||||
}
|
||||
|
||||
/// Refuse a plain-path mutation whose target `slug` resolves to a
|
||||
/// library-projected record (§2.7). The command boundaries that key by slug
|
||||
/// — `delete_persona` and snapshot/team import — call this on the RAW
|
||||
/// keyless store BEFORE any destructive effect, so a projected target fails
|
||||
/// closed with one uniform refusal until §3's state machine routes it. A
|
||||
/// plain or unknown slug (a fresh insert) returns `Ok`.
|
||||
pub(crate) fn reject_projected_slug(
|
||||
records: &[ManagedAgentRecord],
|
||||
slug: &str,
|
||||
) -> Result<(), String> {
|
||||
if Self::for_slug(records, slug) == Self::LibraryProjected {
|
||||
return Err(format!(
|
||||
"persona {slug} is a library projection: route it through the library, \
|
||||
not a plain persona save (§2.7)"
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
/// The shared-definition slots (§2.2 allowlist) of a record, borrowed for
|
||||
/// equality comparison. A plain persona save may freely change a projected
|
||||
/// record's SCOPE-LOCAL fields (`is_active`, `env_vars`, timestamps), but
|
||||
/// changing any of these library-authoritative slots is a shared-definition
|
||||
/// edit that must route through the library — so the merge seam compares only
|
||||
/// this fingerprint, never the whole record. The field set mirrors
|
||||
/// [`SharedDefinition`](crate::managed_agents::library::SharedDefinition), the
|
||||
/// sole writer of these slots.
|
||||
#[derive(PartialEq)]
|
||||
struct SharedSlotFingerprint<'a> {
|
||||
display_name: &'a Option<String>,
|
||||
avatar_url: &'a Option<String>,
|
||||
system_prompt: &'a Option<String>,
|
||||
runtime: &'a Option<String>,
|
||||
model: &'a Option<String>,
|
||||
provider: &'a Option<String>,
|
||||
name_pool: &'a [String],
|
||||
respond_to: &'a Option<String>,
|
||||
respond_to_allowlist: &'a [String],
|
||||
parallelism: &'a Option<u32>,
|
||||
}
|
||||
|
||||
impl<'a> SharedSlotFingerprint<'a> {
|
||||
fn of(record: &'a ManagedAgentRecord) -> Self {
|
||||
Self {
|
||||
display_name: &record.display_name,
|
||||
avatar_url: &record.avatar_url,
|
||||
system_prompt: &record.system_prompt,
|
||||
runtime: &record.runtime,
|
||||
model: &record.model,
|
||||
provider: &record.provider,
|
||||
name_pool: &record.name_pool,
|
||||
respond_to: &record.definition_respond_to,
|
||||
respond_to_allowlist: &record.definition_respond_to_allowlist,
|
||||
parallelism: &record.definition_parallelism,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Build the definition half of a persona save so that every field living only
|
||||
@@ -568,17 +642,19 @@ impl MutationRoute {
|
||||
/// record must not be mutated by this plain path:
|
||||
/// - a projected record absent from `views` is a deletion that must advance the
|
||||
/// §3.4 `ExcludePending` state machine, not silently drop the row;
|
||||
/// - a `views` entry that would change a projected record's shared slots is a
|
||||
/// shared-definition edit (ruling 3a) or a library-linked inbound upsert
|
||||
/// (P4-C2) that must route through the library, not overwrite the local cache
|
||||
/// with no revision.
|
||||
/// - a `views` entry that would change a projected record's SHARED slots (the
|
||||
/// [`shared_slot_fingerprint`] allowlist) is a shared-definition edit (ruling
|
||||
/// 3a) or a library-linked inbound upsert (P4-C2) that must route through the
|
||||
/// library, not overwrite the local cache with no revision.
|
||||
///
|
||||
/// Both fail closed until §3 wires the state machine. A view that leaves a
|
||||
/// projected record unchanged (an unrelated writer re-passing the projected
|
||||
/// view, stripped of its metadata) is not a mutation and rides through intact —
|
||||
/// the reason `library_ref` survives every writer above. No production path
|
||||
/// authors `library_ref` at Phase 0, so the projected branch is an unreachable
|
||||
/// guard rail until §3 populates it.
|
||||
/// Both fail closed. A view that touches only a projected record's SCOPE-LOCAL
|
||||
/// fields (`is_active`, `env_vars`, timestamps) leaves the shared fingerprint
|
||||
/// unchanged and rides through the plain path — the reason a legitimate local
|
||||
/// toggle on a projected agent is not blocked. This save-seam guard is
|
||||
/// defense-in-depth: each command boundary (delete, inbound upsert/tombstone,
|
||||
/// snapshot/team import) runs its own [`MutationRoute::for_slug`] preflight
|
||||
/// BEFORE any destructive effect, so a projected target never reaches a
|
||||
/// half-applied state even though the seam would also reject it here.
|
||||
///
|
||||
/// [`LibraryProjected`]: MutationRoute::LibraryProjected
|
||||
fn merge_preserving_definitions(
|
||||
@@ -597,17 +673,24 @@ fn merge_preserving_definitions(
|
||||
if MutationRoute::for_record(Some(&record)) == MutationRoute::LibraryProjected {
|
||||
let mut candidate = record.clone();
|
||||
candidate.apply_definition_view(view);
|
||||
if candidate != record {
|
||||
if SharedSlotFingerprint::of(&candidate) != SharedSlotFingerprint::of(&record) {
|
||||
return Err(format!(
|
||||
"persona '{}' is a library projection: shared-content edits must \
|
||||
route through the library, not a plain persona save (§2.7)",
|
||||
view.id
|
||||
));
|
||||
}
|
||||
// Only scope-local slots (`is_active`, `env_vars`,
|
||||
// timestamps) changed — apply them. `candidate` carries the
|
||||
// edit and keeps the projection metadata intact, because
|
||||
// `apply_definition_view` never writes `library_ref` or its
|
||||
// siblings. A local toggle on a shared agent must take
|
||||
// effect, not be silently dropped.
|
||||
merged.push(candidate);
|
||||
} else {
|
||||
record.apply_definition_view(view);
|
||||
merged.push(record);
|
||||
}
|
||||
merged.push(record);
|
||||
}
|
||||
None => merged.push(view.clone().into_agent_record()),
|
||||
}
|
||||
|
||||
@@ -617,6 +617,69 @@ fn mutation_route_classifies_projected_plain_and_unknown_slugs() {
|
||||
);
|
||||
}
|
||||
|
||||
/// `for_persona_d_tag` is the inbound routing key, and it is deliberately NOT
|
||||
/// `for_slug`. A team-sourced projected persona derives its d-tag from
|
||||
/// `source_team_persona_slug`, which differs from its UUID `slug`/`id`. The
|
||||
/// inbound apply/tombstone arms match a local record by `persona_d_tag`, so the
|
||||
/// preflight MUST route on the same derivation — a slug-only check would read
|
||||
/// the very same inbound event as `Plain` and let it overwrite or delete the
|
||||
/// projection. This pins that divergence: the projection is found by its d-tag
|
||||
/// but NOT by its slug, and an unknown d-tag is a fresh insert (`Plain`).
|
||||
#[test]
|
||||
fn mutation_route_for_persona_d_tag_catches_projected_team_slug_that_for_slug_misses() {
|
||||
let mut record = projected_record("uuid-team-persona", 5);
|
||||
// Team-sourced: the d-tag derives from the pack slug, not the UUID id/slug.
|
||||
record.source_team_persona_slug = Some("codereviewer".to_string());
|
||||
let records = vec![record];
|
||||
|
||||
// The inbound key (d-tag) finds the projection.
|
||||
assert_eq!(
|
||||
super::MutationRoute::for_persona_d_tag(&records, "codereviewer"),
|
||||
super::MutationRoute::LibraryProjected,
|
||||
);
|
||||
// The slug key does NOT — this is exactly why inbound must not use for_slug.
|
||||
assert_eq!(
|
||||
super::MutationRoute::for_slug(&records, "codereviewer"),
|
||||
super::MutationRoute::Plain,
|
||||
);
|
||||
// A d-tag matching no record is a fresh insert: Plain.
|
||||
assert_eq!(
|
||||
super::MutationRoute::for_persona_d_tag(&records, "unknown"),
|
||||
super::MutationRoute::Plain,
|
||||
);
|
||||
}
|
||||
|
||||
/// The fingerprint narrowing (F3): a plain persona save may freely change a
|
||||
/// projected record's SCOPE-LOCAL fields (`is_active`, `env_vars`) — these are
|
||||
/// not library-authoritative, so the shared fingerprint is unchanged and the
|
||||
/// edit rides through the seam with the projection metadata intact. Only the
|
||||
/// shared-definition slots are gated (see the reject test above). Without the
|
||||
/// narrowing, a whole-record compare would wrongly block a legitimate local
|
||||
/// activation toggle or env edit on a shared agent.
|
||||
#[test]
|
||||
fn merge_preserving_save_allows_scope_local_edit_on_projected_record() {
|
||||
let existing = vec![projected_record("shared", 3)];
|
||||
let mut view = existing[0].to_definition_view().expect("view");
|
||||
view.is_active = !view.is_active;
|
||||
view.env_vars =
|
||||
std::collections::BTreeMap::from([("API_KEY".to_string(), "value".to_string())]);
|
||||
|
||||
let saved = merge_preserving_definitions(existing, &[view])
|
||||
.expect("a scope-local edit on a projection must pass the seam");
|
||||
assert_projection_survived(&saved, "scope_local_edit");
|
||||
|
||||
let shared = saved
|
||||
.iter()
|
||||
.find(|record| record.slug.as_deref() == Some("shared"))
|
||||
.expect("projected record survives the scope-local edit");
|
||||
assert!(!shared.is_active, "activation toggle must be applied");
|
||||
assert_eq!(
|
||||
shared.env_vars.get("API_KEY").map(String::as_str),
|
||||
Some("value"),
|
||||
"env edit must be applied",
|
||||
);
|
||||
}
|
||||
|
||||
/// The same guarantee through the on-disk `_at` seam — proving the storage
|
||||
/// layer and the built-in-merge write-back preserve the metadata too — plus the
|
||||
/// §2.7 read-side exposure (P4-C1): `load_persona_views_at` surfaces the
|
||||
@@ -671,3 +734,81 @@ fn save_personas_at_preserves_library_metadata_on_disk() {
|
||||
"a plain persona must surface no library metadata",
|
||||
);
|
||||
}
|
||||
|
||||
/// F3 per-command-path preflight (§2.7): every command boundary reads its route
|
||||
/// from the RAW keyless store it loads, never from an in-memory vector or an
|
||||
/// inbound/import view (`library_ref` is not view-carried). `delete_persona` and
|
||||
/// both snapshot/team imports load `load_agent_definitions[_at]` then route on
|
||||
/// `for_slug`; both inbound arms load the same store then route on
|
||||
/// `for_persona_d_tag`. The command bodies take a concrete `AppHandle<Wry>`, so
|
||||
/// they cannot be driven by the `MockRuntime` harness — this binds the decision
|
||||
/// each one consults to the real serializer path instead: a projected keyless
|
||||
/// definition must survive the `pubkey.is_empty()` definition filter AND the
|
||||
/// JSON round-trip and STILL classify `LibraryProjected`, or a preflight would
|
||||
/// wave a projected target onto the destructive plain path. A team-sourced
|
||||
/// projection exercises the inbound d-tag key (derived from
|
||||
/// `source_team_persona_slug`, not the UUID slug) end-to-end through disk. The
|
||||
/// refusal-precedes-all-effects ordering is by construction at each call site
|
||||
/// (verified by source inspection) — the preflight is the first statement under
|
||||
/// the store lock, before any runtime stop, cascade delete, retention write, or
|
||||
/// keyed-record save.
|
||||
#[test]
|
||||
fn command_preflight_routes_projected_record_off_the_reloaded_raw_store() {
|
||||
let dir = tempfile::tempdir().expect("temp dir");
|
||||
let definitions_dir = dir.path();
|
||||
|
||||
let mut projected = projected_record("uuid-shared", 3);
|
||||
// Team-sourced: the inbound arms match by `persona_d_tag`, which derives
|
||||
// from the pack slug, not the UUID id/slug.
|
||||
projected.source_team_persona_slug = Some("codereviewer".to_string());
|
||||
save_agent_definitions_at(
|
||||
definitions_dir,
|
||||
&[
|
||||
projected,
|
||||
custom_persona("custom:plain", "Plain").into_agent_record(),
|
||||
],
|
||||
)
|
||||
.expect("seed raw store");
|
||||
|
||||
// The exact load every preflight runs (`load_agent_definitions[_at]` share
|
||||
// one body: read the store, retain keyless definitions). The projected
|
||||
// keyless definition must survive it, or the route would silently be Plain.
|
||||
let raw = load_agent_definitions_at(definitions_dir).expect("reload raw store");
|
||||
assert!(
|
||||
raw.iter().any(|r| r.slug.as_deref() == Some("uuid-shared")),
|
||||
"projected keyless definition must survive the definition filter",
|
||||
);
|
||||
|
||||
// delete + snapshot/team import key: route on the UUID slug.
|
||||
assert_eq!(
|
||||
super::MutationRoute::for_slug(&raw, "uuid-shared"),
|
||||
super::MutationRoute::LibraryProjected,
|
||||
"delete/import preflight must refuse a projected slug",
|
||||
);
|
||||
assert_eq!(
|
||||
super::MutationRoute::for_slug(&raw, "custom:plain"),
|
||||
super::MutationRoute::Plain,
|
||||
"a plain persona is not a projection",
|
||||
);
|
||||
|
||||
// inbound upsert/tombstone key: route on the team-derived d-tag. A slug-only
|
||||
// check would MISS it (the d-tag is not the UUID slug) — which is exactly
|
||||
// why the arms must use for_persona_d_tag.
|
||||
let projected_view = raw
|
||||
.iter()
|
||||
.find(|r| r.slug.as_deref() == Some("uuid-shared"))
|
||||
.and_then(|r| r.to_definition_view())
|
||||
.expect("projected view");
|
||||
let d_tag = crate::managed_agents::persona_events::persona_d_tag(&projected_view);
|
||||
assert_eq!(d_tag, "codereviewer");
|
||||
assert_eq!(
|
||||
super::MutationRoute::for_persona_d_tag(&raw, &d_tag),
|
||||
super::MutationRoute::LibraryProjected,
|
||||
"inbound preflight must refuse a projected target by its d-tag",
|
||||
);
|
||||
assert_eq!(
|
||||
super::MutationRoute::for_slug(&raw, &d_tag),
|
||||
super::MutationRoute::Plain,
|
||||
"the d-tag is not the slug — a slug-only inbound check would miss it",
|
||||
);
|
||||
}
|
||||
|
||||
@@ -583,42 +583,78 @@ fn empty_prompt_folds_to_none() {
|
||||
assert_eq!(persona.into_agent_record().system_prompt, None);
|
||||
}
|
||||
|
||||
/// P13-I3 / invariant 4: a legacy head-serialized record (no `library_ref`,
|
||||
/// no `library_applied_revision`, no `last_completed_deploy_attempt_id`) reads
|
||||
/// with all three `None` and re-serializes WITHOUT resurrecting the keys —
|
||||
/// `skip_serializing_if` keeps it byte-identical to head, so a device that
|
||||
/// never touched the library is untouched by the fold.
|
||||
/// P13-I3 / invariant 4: a legacy device that never touched the library must
|
||||
/// be byte-untouched by the fold. This drives the REAL store save→load→save
|
||||
/// path (`save_agent_definitions_at` / `load_agent_definitions_at`, the
|
||||
/// serializer `save_personas` funnels through), not an in-memory
|
||||
/// `to_value`, so it catches field ordering, formatting, and any unrelated
|
||||
/// default-field drift that a value-only comparison misses.
|
||||
///
|
||||
/// The baseline is the exact bytes the pinned-head serializer produces for a
|
||||
/// legacy definition: `storage.rs` (the serializer — `serde_json::to_vec_pretty`
|
||||
/// over the record vector) is byte-identical base→HEAD, and the three new fields
|
||||
/// are `#[serde(skip_serializing_if = "Option::is_none")]`, so a legacy record
|
||||
/// with all three `None` serializes exactly as it did at head. The test proves
|
||||
/// (1) the three keys never appear on disk and (2) load→save is a byte-fixpoint
|
||||
/// on that legacy shape — a one-field regression (dropping a `skip_serializing_if`,
|
||||
/// or shedding/adding a field) breaks the fixpoint or the key-absence assertion.
|
||||
#[test]
|
||||
fn legacy_record_without_deploy_provenance_round_trips_byte_identically() {
|
||||
let legacy = r#"{
|
||||
"pubkey": "abcd1234",
|
||||
"name": "test-agent",
|
||||
"private_key_nsec": "nsec1fake",
|
||||
"relay_url": "wss://localhost:3000",
|
||||
"acp_command": "buzz-acp",
|
||||
"agent_command": "goose",
|
||||
"agent_args": [],
|
||||
"mcp_command": "",
|
||||
"turn_timeout_seconds": 320,
|
||||
"system_prompt": null,
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"updated_at": "2026-01-01T00:00:00Z",
|
||||
"last_started_at": null,
|
||||
"last_stopped_at": null,
|
||||
"last_exit_code": null,
|
||||
"last_error": null
|
||||
}"#;
|
||||
let record: ManagedAgentRecord =
|
||||
serde_json::from_str(legacy).expect("legacy record deserializes");
|
||||
assert_eq!(record.library_ref, None);
|
||||
assert_eq!(record.library_applied_revision, None);
|
||||
assert_eq!(record.last_completed_deploy_attempt_id, None);
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let definitions_dir = dir.path();
|
||||
|
||||
let reserialized = serde_json::to_value(&record).expect("serialize");
|
||||
let object = reserialized.as_object().expect("record is an object");
|
||||
assert!(!object.contains_key("library_ref"));
|
||||
assert!(!object.contains_key("library_applied_revision"));
|
||||
assert!(!object.contains_key("last_completed_deploy_attempt_id"));
|
||||
// A legacy keyless definition: the shape `into_agent_record` produces and
|
||||
// `save_agent_definitions_at` persists, with all three library fields `None`
|
||||
// (never authored before §3). Keyless so it is a definition, not an instance.
|
||||
let legacy = sample_persona().into_agent_record();
|
||||
assert!(legacy.pubkey.is_empty(), "definition is keyless");
|
||||
assert_eq!(legacy.library_ref, None);
|
||||
assert_eq!(legacy.library_applied_revision, None);
|
||||
assert_eq!(legacy.last_completed_deploy_attempt_id, None);
|
||||
|
||||
// Baseline = the exact bytes the (base==HEAD) serializer writes for this
|
||||
// legacy record. Captured from the first real store write, this is the
|
||||
// pinned-head on-disk form: any drift below is measured against it.
|
||||
crate::managed_agents::storage::save_agent_definitions_at(
|
||||
definitions_dir,
|
||||
std::slice::from_ref(&legacy),
|
||||
)
|
||||
.expect("write legacy store");
|
||||
let store_path = crate::managed_agents::storage::managed_agents_store_path_at(definitions_dir);
|
||||
let baseline = std::fs::read(&store_path).expect("read baseline bytes");
|
||||
|
||||
// The three library keys must never reach disk for a legacy record.
|
||||
let baseline_text = std::str::from_utf8(&baseline).expect("store is utf-8");
|
||||
assert!(
|
||||
!baseline_text.contains("library_ref"),
|
||||
"no library_ref on disk"
|
||||
);
|
||||
assert!(
|
||||
!baseline_text.contains("library_applied_revision"),
|
||||
"no library_applied_revision on disk"
|
||||
);
|
||||
assert!(
|
||||
!baseline_text.contains("last_completed_deploy_attempt_id"),
|
||||
"no last_completed_deploy_attempt_id on disk"
|
||||
);
|
||||
|
||||
// Fixpoint: load the store back and re-save it through the same real path.
|
||||
// A legacy device that never touches the library re-writes byte-identical
|
||||
// bytes — no key resurrection, no field reordering, no default drift.
|
||||
let reloaded = crate::managed_agents::storage::load_agent_definitions_at(definitions_dir)
|
||||
.expect("reload store");
|
||||
assert_eq!(reloaded.len(), 1, "one definition round-trips");
|
||||
assert_eq!(reloaded[0].library_ref, None);
|
||||
assert_eq!(reloaded[0].library_applied_revision, None);
|
||||
assert_eq!(reloaded[0].last_completed_deploy_attempt_id, None);
|
||||
|
||||
crate::managed_agents::storage::save_agent_definitions_at(definitions_dir, &reloaded)
|
||||
.expect("re-save store");
|
||||
let after = std::fs::read(&store_path).expect("read re-saved bytes");
|
||||
assert_eq!(
|
||||
after, baseline,
|
||||
"load→save must be a byte-exact fixpoint on a legacy record"
|
||||
);
|
||||
}
|
||||
|
||||
/// P14-I2: the deploy-attempt stamp is not projection metadata, so a fresh
|
||||
|
||||
Reference in New Issue
Block a user