fix(desktop): propagate respond_to changes from persona to linked agent instances - #6039
fix(desktop): propagate respond_to changes from persona to linked agent instances#6039BradGroux wants to merge 2 commits into
Conversation
themiguelamador
left a comment
There was a problem hiding this comment.
I found three issues with the propagation approach:
- P1 — A persona-template edit overwrote every linked instance's explicit access policy. Instance
respond_tovalues can be deliberate overrides; the codebase preserves those overrides during resnapshot and surfaces persona drift separately. With this patch, changing a persona from owner-only to anyone silently widens even an instance intentionally pinned owner-only. That is the security-sensitive direction the fix must not introduce. - P2 — Invalid definition modes were silently converted to owner-only.
RespondTo::parse_wire(wire).ok().unwrap_or_default()discards the parse error even though the type's documented invariant is to fail loudly rather than launch with a different audience. - P2 — The changed persistence behavior had no regression test. The cited existing resnapshot test exercises a different path and cannot catch either the broad overwrite or the original profile-edit omission.
The reported flow already has the correct scope boundary: after saving the persona, personaManagedAgentUpdate builds a patch for the single linked agent whose profile the user edited. I fixed #6026 there in commit a49142662 on review/pr-6039-fix, including null-definition → owner-only handling and exact allowlist synchronization, and removed the unsafe global backend propagation.
Verification: full Desktop frontend suite (4,956 passed); pnpm typecheck; pnpm check (clean for changed files; four pre-existing advisory diagnostics remain elsewhere); focused mapping regressions; git diff --check.
a072331 to
84a4707
Compare
|
Thanks @themiguelamador — the security-sensitive direction was the key insight. Your fixes are folded into this branch (commit
|
…nt instances When an agent's inbound author gate (respond_to) was changed via the persona edit dialog, the definition fields (definition_respond_to, definition_respond_to_allowlist) were saved correctly, but the runtime fields (respond_to, respond_to_allowlist) that build_respond_to_env reads at spawn were never updated. The propagation block in update_persona_with only fired on avatar or display_name changes — behavioral changes were silently dropped. The agent kept booting with the stale gate (e.g. owner-only) across restarts while the Desktop UI showed the updated value (e.g. allowlist). No error was surfaced. The issue was security-relevant: a user who widened access saw the wider setting in the UI but the agent silently kept the narrower gate. The fix detects behavior group changes (mode or allowlist) and propagates the definition's respond_to onto each linked instance's runtime fields in the same record-load-and-save cycle that already handles avatar and name propagation. The persona's wire-shape Option<String> is parsed to the typed RespondTo enum; an absent value falls back to OwnerOnly (the default). This does not change apply_persona_snapshot, which intentionally preserves instance-level overrides during re-snapshot at start/restore. The fix is scoped to the persona edit path only. Closes block#6026 Co-authored-by: Brad Groux <brad@digitalmeld.communities.buzz.xyz> Signed-off-by: Brad Groux <brad@digitalmeld.communities.buzz.xyz> Signed-off-by: dm-builder <f01486f036641ccb52c11bb1e0ff2346ea89a8b4e3b49cd772249948f6fcbae6@digitalmeld.communities.buzz.xyz>
The Rust-side propagation overwrote every linked instance's respond_to when the persona was edited, including instances with deliberate per-instance overrides. The security-sensitive direction (widening owner-only to anyone) was silently applied to all linked agents. The correct scope is the frontend's personaManagedAgentUpdate, which builds a patch for the single linked agent whose profile the user edited. Add respondTo and respondToAllowlist fields to the update function: sync the persona's mode (null defaults to owner-only) and sync the allowlist only when mode is allowlist (clear otherwise). Remove the behavior_changed block from the Rust update_persona_with loop entirely — the frontend handles it for the one linked agent. Add regression tests for mode sync, allowlist sync, allowlist clearing, and null-to-owner-only fallback. Addresses themiguelamador's review feedback. Co-authored-by: Brad Groux <brad@digitalmeld.communities.buzz.xyz> Signed-off-by: Brad Groux <brad@digitalmeld.communities.buzz.xyz>
e671f5c to
a141446
Compare
|
Rebased on latest main (a282e06). Resolved a conflict in
|
What users saw
Changing an agent's inbound author gate (e.g. "Only me" → "Allowlist") in the Desktop persona edit dialog saved the definition fields (
definition_respond_to,definition_respond_to_allowlist) but never reached the runtime fields (respond_to,respond_to_allowlist) thatbuild_respond_to_envreads at spawn. The agent kept booting with the stale gate (e.g.owner-only) across restarts while the UI showed the updated value (allowlist). No error was surfaced.The issue was security-relevant in one direction: a user who widened access saw the wider setting in the UI, but the agent silently kept the narrower gate and dropped messages from allowlisted identities.
Why it happened
The persona edit command (
update_persona_withindesktop/src-tauri/src/commands/personas/update.rs) saves the definition viaapply_persona_behavior, then propagates changes to linked agent instances. But the propagation block only fired whenavatar_changed || name_changed— behavioral changes (respond_tomode + allowlist) were not in the trigger condition, so arespond_to-only edit saved the definition and skipped the runtime record entirely.The runtime record's
respond_to/respond_to_allowlistfields are the onesbuild_respond_to_envreads at spawn (desktop/src-tauri/src/managed_agents/runtime.rs:769). Thedefinition_*fields are stored on the record for catalog/publish purposes but are never consulted at spawn time.What changed
Added behavior-change detection in
update_persona_with: beforeapply_persona_behaviorruns, the pre-editrespond_toandrespond_to_allowlistare captured. After the apply, abehavior_changedflag compares old vs. new. When the flag is set, the propagation block (which already loads linked agent records for avatar/name changes) also copies the definition'srespond_toonto each linked instance's runtimerespond_to/respond_to_allowlistfields.The persona stores
respond_toin wire shape (Option<String>); the fix parses it to the typedRespondToenum viaRespondTo::parse_wire. An absent value falls back toOwnerOnly(the default). When the mode is notAllowlist, the allowlist is cleared (stale entries must not survive a mode toggle).This does not change
apply_persona_snapshot, which intentionally preserves instance-level overrides during re-snapshot at start/restore. The fix is scoped to the persona edit path only, matching the existing pattern for avatar and name propagation.Testing
buzz-desktoplib suite: 2,440 passed, 0 failed, 15 ignoredcargo fmtcleancargo clippyclean (no warnings)The existing test
resnapshot_does_not_clobber_record_quad_with_definition_absent_quad(which verifies thatapply_persona_snapshotdoes NOT overwrite instance-levelrespond_towith definition defaults) still passes — confirming the fix respects instance-level overrides outside the persona edit path.Closes #6026