Skip to content

fix(desktop): cascade persona respond_to edits to linked inheriting instances (#2501) - #4115

Open
iroiro147 wants to merge 1 commit into
block:mainfrom
iroiro147:claude/issue2501-persona-behavior-cascade-20260801
Open

fix(desktop): cascade persona respond_to edits to linked inheriting instances (#2501)#4115
iroiro147 wants to merge 1 commit into
block:mainfrom
iroiro147:claude/issue2501-persona-behavior-cascade-20260801

Conversation

@iroiro147

Copy link
Copy Markdown
Contributor

Summary

  • Fixes the broken "Who can talk to this agent" edit path for persona/definition-backed agents: the persona behavior update went only to the definition record, so every already-minted agent instance kept its mint-time respond_to (owner-only by default). The harness boots BUZZ_ACP_RESPOND_TO from the instance record (build_respond_to_env), so UI edits had zero runtime effect — exactly the three-way desync the issue documents (UI says Anyone, definition says definition_respond_to=anyone, harness boots owner-only).
  • update_persona_with now cascades behavioral-group edits to linked instance records, using the same discrimination rule the existing display_name cascade applies (pool-named instances carry an override and are skipped): an instance whose respond_to still equals the pre-edit definition value was inheriting → it adopts the new mode/allowlist/parallelism; an instance carrying a different value holds an explicit pin → preserved untouched. Definition mirror fields on every linked instance refresh either way.
  • Fail-loudly parity with mint: unknown mode strings error out instead of silently rewriting inheriting instances; absent definition parallelism does not stomp an instance's pool width.

Root cause

create_managed_agent mints the instance record.respond_to once via resolve_mint_behavioral_defaults (explicit input > definition default > owner-only). After that the instance record is the only thing buzz-acp reads at spawn. The persona edit path (update_personaapply_persona_behavior) updated the definition only — the cascade to linked instances existed for avatar_url/display_name (Issue pattern) but was never built for the behavioral group, so the instance record froze at mint time. The issue reporter's workaround (hand-editing managed-agents.json) worked precisely because the store is authoritative — the write path was the gap.

Changes

  • desktop/src-tauri/src/commands/personas/update.rs: capture pre-edit behavior signature; new propagate_persona_behavior helper (mirrors propagate_persona_name_rename) applying the cascade with override discrimination + mirror refresh; called from update_persona_with when the behavior signature changed.
  • desktop/src-tauri/src/commands/personas/update/behavior_cascade_tests.rs: 6 pinned regression tests — inheriting cascade, pinned-override preservation, mixed fleet per-instance semantics, definition-clear resets to default, unknown mode fails loudly without half-apply, absent parallelism preserves instance width.

Verification

  • cargo test --lib — 2094 passed, 0 failed (includes the 6 new tests).
  • pnpm typecheck — clean.

Fixes #2501

…nstances (block#2501)

A persona definition's "Who can talk to this agent" edit updated only the
definition record; every already-minted agent instance kept its mint-time
respond_to (owner-only by default), because the harness boots the gate from
the instance record (build_respond_to_env) and no write path ever propagated
behavior changes from the definition. Manual managed-agents.json edits worked,
confirming the store was authoritative and the write path was the gap.

update_persona_with now cascades behavioral-group edits to linked instance
records, with the same discrimination rule the display_name cascade uses:
- An instance whose respond_to still equals the PRE-edit definition value
  was inheriting → it adopts the new definition mode/allowlist/parallelism.
- An instance carrying a different value holds an explicit instance-level
  pin → preserved untouched (parity with the pool-named-instance rule).
- Every linked instance's definition mirror fields refresh either way, so
  future mint/inspect paths see the current definition bytes.

Absent definition parallelism does not stomp an instance's pool width;
unknown mode strings fail loudly rather than rewriting inheriting instances
to a default the author didn't choose (same contract as mint).

Adds 6 regression tests: inheriting cascade, pinned-override preservation,
mixed fleet, definition-clear resets to default, unknown mode fail-loudly,
and absent parallelism preservation. cargo test --lib: 2094 pass.

Signed-off-by: Sarthak Singh <sarthak.singh@juspay.in>
@georgerous

Copy link
Copy Markdown

Nice — this is the cascade display_name/avatar_url already had and the behavioral group never did, and putting it in update_persona_with is the right layer (the store is authoritative, which is exactly why the hand-edit held). Read the diff. Two things I'd want closed before it lands, plus a scope note.

1. No empty-allowlist guard on the inherit branch — it regenerates the Allowlist+[] state the mint path rejects.

The inherit branch copies the definition allowlist verbatim:

record.respond_to = match persona.respond_to.as_deref() {
    Some(wire) => RespondTo::parse_wire(wire)?,
    None => RespondTo::default(),
};
record.respond_to_allowlist = persona.respond_to_allowlist.clone();

parse_wire("allowlist") succeeds (known mode), so a definition sitting in respond_to = allowlist with an empty respond_to_allowlist cascades Allowlist + [] onto every inheriting instance. That definition state is reachable today, not hypothetical: validate_respond_to_allowlist(&[]) returns Ok(vec![]) instead of erroring, and it's exactly what the person-picker produces when the mode is written but the principals aren't — the first of the two defects in #2501 (mode persists, members don't). resolve_mint_behavioral_defaults and apply_persona_behavior both reject Allowlist+empty, so this cascade writes a record neither the mint path nor a later behavior edit would ever produce. Same hole I flagged on #2505 (comment 5135752902); same ask: mirror the mint guard and skip that instance, don't fail — leave it on its current gate rather than adopt an unusable one. None of the six tests cover it (behavior_edit_cascades_per_instance uses a non-empty vec!["d"…]); an allowlist+[] case would pin it.

2. The inheritance discriminator is mode-only, so it clobbers a same-mode allowlist pin.

if record.respond_to == old_mode {

old_mode is the definition's pre-edit mode; the allowlist isn't part of the discriminant. So an instance pinned to allowlist + [X] while the definition mode was also allowlist reads as "still inheriting," and its respond_to_allowlist gets overwritten by the definition's. A per-instance allowlist pin under an unchanged mode can't be expressed — and that's precisely the shape of the current working workaround (per-instance EditRespondToDialog, which writes the instance record directly). propagate_persona_name_rename's pool-name rule survives this only because a rename always changes the discriminant; a same-mode allowlist edit doesn't. behavior_edit_preserves_instance_override only exercises a different mode (OwnerOnly pin vs anyone edit); the interesting case is same-mode / different-allowlist.

3. Scope / coordination with #2505. This is backend-only, so defect 1 (person-picker never persisting respond_to_allowlist on either record — MaxWynnDev's repro) is still open: an allowlist persona configured through the UI can't reach the harness with its members regardless of this cascade. Worth stating so this doesn't read as a full #2501 fix — it closes the mode desync (defect 2) cleanly, but #2501 still needs the frontend fix. How do you see this coordinating with #2505, which fixes an overlapping seam from the TS side?

Happy to build and run an end-to-end regen check (owner-only → allowlist-with-members, mixed inheriting/pinned fleet, restart + re-mint) once the empty-allowlist skip is in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] <Desktop: agent respond_to / allowlist edits don't persist — buzz-acp always starts with owner-only>

2 participants