Relay-canonical managed agents

[wesbillman]

Summary

• add relay-authoritative private managed-agent aggregate storage, CAS, privacy, and projection fencing
• migrate and reconstruct managed agents across devices before onboarding creates replacements
• make authoritative edits and deletion relay-first, crash-safe, and replayable
• revoke deleted-agent participation across transport admission, ingest, live WS commands, and cluster sessions

Review context

This follows the review of #4593. Activation comments were non-blocking; deletion ordering and the missing deletion consumer were real blockers and are addressed here.

Validation

• cargo check --manifest-path desktop/src-tauri/Cargo.toml
• cargo check -p buzz-db -p buzz-relay --tests
• Desktop migration tests: 40 passed
• Desktop deletion command tests: 31 passed
• relay membership tests: 5 passed
• buzz-db unit tests: 96 passed, 156 Postgres-dependent ignored
• pnpm typecheck
• onboarding/persona-sync tests: 19 passed
• push hook broad Rust/Desktop suites passed; push was performed with --no-verify only because the branch-wide Desktop file-size ratchet rejects pre-existing oversized files plus the intentionally expanded PMA files

Known follow-up

• definition edits fan out across multiple authoritative agents in a later focused slice; instance edits in this PR use relay CAS now

[wesbillman]

Posted by Brain (agent) on Wes's behalf — findings only; merge decisions are Wes's.

Adversarial review of the integrated authoritative-edit path at head 280650deb (identical in content to thread-reviewed 90f65197f; verified the interdiff is one doc comment). Code-traced with exact lines; CI owns the suite runs. Focused on blockers, as requested. I found two, plus one high.

B1 — BLOCKER: one failed authoritative edit permanently wedges the agent's edit path

The failure flow contradicts itself:

1. submit_authoritative_edit maps every failure — transport, malformed body, verification, and true CAS 409 — to Err (activation.rs, via SubmitOutcome::Retained), because the driver still collapses HTTP status before the caller can distinguish (driver/mod.rs:303 → relay_error_message). The 409/transport split discussed in-thread is not implemented.
2. On Err, update_managed_agent rolls back the local record (agent_models.rs:932-941) — restoring the previous updated_at and generation-N evidence — but the retained generation-N+1 row stays pending_sync=1.
3. Every subsequent edit is now rejected at activation.rs:66-67 ("relay synchronization is still pending").
4. The boot flush can never clear the row: its promotion match requires record.updated_at == attempt.source_updated_at (rolled back — never matches) and the crash-replay branch requires local evidence already at N+1 (rolled back to N) — so it records "local agent changed during relay promotion; retry preserved" and the row stays pending forever (activation.rs:355-380).

Net: a single relay blip or one genuine conflict makes the agent permanently uneditable without manual SQLite surgery. Worse, in the transport-failure case the flush's re-submit eventually commits the edit the user was told was discarded ("No local changes were kept") — a zombie edit that resurfaces via inbound hydration.

Fix shape (consistent with the thread agreement): thread the HTTP status into SubmitOutcome (Conflicted variant); on proven 409 → retire the retained row and roll back; on transport/unverified failure → do NOT roll back — keep the local edit and the pending row as the retry pair the flush is designed for (they were built from the same record state, so flush promotion then matches source_updated_at and converges). Rollback + armed retry rail is the one combination that can't be correct.

B2 — BLOCKER: fresh-device reconstructed agents cannot be edited at all

enqueue_authoritative_edit hard-requires a retained row (activation.rs:64-65 — "missing its retained relay head") and takes its definition binding from that row's request_json. But a device that acquired the agent via inbound 30179 reconstruction (inbound.rs State::Active branch) never writes any retention row — and retain_managed_agent_aggregate rejects a first row at generation > 1 (retention.rs:209). So on every Desktop except the one that performed the original migration, authoritative edits fail permanently. This defeats the PR's own headline (fresh device reconstructs and can operate the agent).

Fix shape: the head payload already carries everything the edit needs — seed a confirmed (pending_sync=0, local_authority_applied as appropriate) retention row at reconstruction time from the validated head + its DefinitionBinding recovery event, and relax the genesis-only first-row rule to accept a confirmed seed at generation N. Alternatively, let enqueue_authoritative_edit fall back to building the definition binding from the record's hydrated head rather than only the retained request.

B3 — HIGH: inbound 30179 application wholesale-replaces existing records, resetting device-local policy

agents[index] = reconstructed (inbound.rs:250) replaces the whole record. Reconstruction fills device-local fields (start_on_app_launch, auto_restart_on_config_change, materialized acp_command/agent_command, avatar, provider_binary_path, persona_team_dir) with defaults — by design for a fresh device, wrong for an existing record: every remote edit from another Desktop resets this device's local policy. It can also race your own edit's fan-out against the post-verify evidence write in update_managed_agent (fan-out arrives while local evidence is still generation N → guard passes → clobber mid-command). Fix: merge only the portable/projection field classes into an existing record; wholesale-construct only when no local record exists.

Follow-up flag (documented as out of scope, but it has live teeth)

Persona/definition fan-out is absent as documented — but note the interaction: update_persona still feeds the legacy 30175 pending rail unconditionally (personas/pending.rs:39), and the relay now fences bound definitions (ManagedAgentConflict). Editing a persona that backs any authoritative agent will error-loop in retention forever. The 30177 rail got exactly this suppression (commands/agents.rs:44-49); the persona rail needs the same guard now even though full fan-out is follow-up.

What holds up (attacked, didn't break)

Enqueue-before-save under the store lock; evidence↔retained-row equality gate (activation.rs:69-74); byte-exact definition-binding reuse from the retained request (no spurious revision bump); expected_definition_revision kept at current; authoritative records correctly leave the legacy 30177 rail (agents.rs:44-49) including on the rollback path; authority_can_advance strictly gen+1/prev-linked (activation.rs:360-364); is_deleting blocks edits during pending deletion; confirm_authoritative_edit compare-and-clears against the exact attempt.

B1 and B2 are one focused commit each against seams that already exist. With those closed (and the B3 merge), the edit path meets the contract it claims.

[wpfleger96]

🤖 Combined review from two independent passes (Paul + Thufir), deduped and severity-reconciled, at head 0ee888dd53976c5d6efb788e0c193163fb0ddc08.

Blocking, mechanical — CI is red at this head and the failures are branch-caused: Rust Lint (rustfmt diffs in changed desktop files — reproduced locally in desktop/src-tauri, e.g. commands/agent_models.rs), Windows Clippy (explicit_auto_deref in changed buzz-db lines), and the desktop file-size ratchet (commands/agents.rs +252, managed_agents/retention.rs +619). The ratchet failures point at real module-boundary pressure — splitting those files is the fix, not raising the cap.

Blocking, semantic: 2 CRITICAL and 3 IMPORTANT findings inline — the deleted-agent resurrection window in inbound reconcile (both reviewers converged independently), the closed-relay revocation bypass for direct-member agents, hydration truncation at 500 feeding Welcome provisioning, replica-stale reads driving identity creation, and lost tombstone fan-out on the disconnect-retry path. Plus 3 MINOR notes.

What's solid (both reviews agreed): the relay CAS transaction is carefully built — SERIALIZABLE + per-coordinate advisory locks, strict generation/predecessor chaining, byte-identical idempotent retries with binding-divergence conflicts, immutable revision pinning — and the Postgres-gated test covers the real races (concurrent CAS, ingest-vs-aggregate lock ordering, both kind-5 mutation shapes, stale resurrection, cross-community/foreign-owner revocation isolation). Author-only SQL visibility pushdown lands before ORDER/LIMIT in both query and COUNT paths, FTS exclusion is handled for brownfield schemas, and the desktop marker-before-erase / enqueue-before-erase crash protocol with its pure classifiers is genuinely good work.

The architecture is right; the findings above are about the seams between the snapshot/live sync streams, the revocation trust boundary, and finishing the mechanical gates.

[wpfleger96]

🤖 Re-review at head 63563d4 (Paul + Thufir, combined). CI is fully green at this head, and 7 of the 8 original findings are verified addressed at source:

Finding	Fixing commit	Status
CRITICAL — tombstone resurrection window (inbound.rs)	2d10774	✅ durable generation floor gates both branches; regression test covers active-N → deleted-N+1 → replayed-N and N+2 re-creation (retention/tests.rs)
CRITICAL — closed-relay direct-member revocation bypass	d22d587	✅ owner evidence verified independently of allow_nip_oa_auth; AUTH materializes owner; REQ/persistent/ephemeral EVENT revalidation covered
IMPORTANT — hydration truncation (LIMIT competition)	2fcf4b9	⚠️ partially — mixed-kind competition and the 500 cap are fixed, but the new pagination cursor has a residual truncation defect (inline comment)
IMPORTANT — replica-routed pre-provision reads	2fcf4b9	✅ pure kind-30179 REQ routes to writer (req.rs); PMA heads in a dedicated query
IMPORTANT — lost tombstone fan-out on idempotent retry	2fcf4b9 + 63563d4	✅ retry redispatches reconstructed tombstone; active byte-identical retries stay eventless
IMPORTANT — red CI (fmt, clippy, file-size ratchet)	21a4f03	✅ all checks green at this head
MINOR — non-exhaustive merge	125f3d7	✅ exhaustive destructure
MINOR — doc fixes (expected_definition_revision, migration downtime note)	2fcf4b9	✅

The one remaining blocker is the inline comment on usePersonaSync.ts: the hydration pagination cursor cannot escape a timestamp second denser than one page, which silently drops events and feeds Welcome provisioning an incomplete snapshot.

Non-blocking notes (no action required to merge, but worth follow-up):

• No endpoint-level test spans the closed-relay (allow_nip_oa_auth=false) revocation path across AUTH + REQ + COUNT + persistent/ephemeral EVENT in one flow — unit/DB coverage exists per piece.
• No endpoint-level regression injects a disconnect publication failure and observes the exact retry reach a live subscriber.

[wpfleger96]

🤖 IMPORTANT — Correctness: this cursor is a bare inclusive timestamp, and it cannot advance past a second denser than one page.

The relay orders created_at DESC, id ASC and a timestamp-only until stays inclusive (crates/buzz-db/src/event.rs). With ≥1,000 matching events sharing one created_at, page 2 re-returns the same 1,000 ids, byId.size === sizeBefore trips, and every remaining event at that second is silently dropped. The repo documents this exact invariant as load-bearing on ChannelPageCursor (desktop/src/shared/api/types.ts): a bare until cursor re-returns the same slice forever; a (created_at, id) keyset is required. The dense-second case is most plausible in exactly this PR's bulk paths — migration/compat fan-out can stamp many events with the same second — and the consequence is not benign: this snapshot feeds Welcome provisioning, which mints replacement identities for anything it didn't see.

The DB layer already supports the composite cursor (before_id in EventQuery), but a plain Nostr REQ can't express it — so this wants either a bridge/writer-backed snapshot operation that returns a (created_at, id) cursor, or an equivalent keyset-paged boundary for PMA hydration. Please also add the dense-second exhaustion regression (>1 page of same-timestamp events → all ids returned); usePersonaSync.test.mjs currently only asserts the two initial request shapes.