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

[iroiro147]

Summary

respond_to / respond_to_allowlist / parallelism edits on a persona definition never reached linked instances: the harness boots respond_to from the instance record (build_respond_to_env), so every running agent silently stayed at its mint-time mode — owner-only by default — whatever the persona said (defect 2 of #2501).

update_persona_with now cascades behavior edits to linked instance records, mirroring propagate_persona_name_rename.

How an instance is judged inheriting vs pinned (shaped by review, see thread):

• An instance is inheriting when its gate equals the pre-edit definition value — mode and allowlist, compared order-insensitively (sorted, not set-wise, so a duplicated principal still reads as a different list). A same-mode allowlist pin therefore does not read as inheritance.
• Both the comparison and the adoption use the effective allowlist (empty for non-allowlist modes), matching apply_persona_behavior's storage asymmetry.
• A definition sitting in allowlist + [] (reachable via the person-picker today; the crash-shape the mint path rejects) skips only the respond_to gate adoption; parallelism still cascades, and mirror fields still refresh.
• A record whose mirrors are unset and whose gate is still the mint default is a pre-fix desynced record and is healed rather than read as a pin (@xtranger51's store bytes are the fixture). To make that marker sound, mint now stamps the definition mirror triple in create_managed_agent — without that, every fresh mint matched the pre-fix shape and a dialog pin could be silently un-pinned after fix(desktop): send complete respond_to wire patch on every allowlist transition #4270 lands (@georgerous's catch).
• Records the cascade actually adopts republish their retained kind:30177 projection (the behavioral triple is in agent_event_content), same contract as the rename cascade ([Bug] Renaming or re-adding personal agents can desynchronise identity and break @mentions #2423). Mirror-only refreshes do not retain.
• An unknown definition mode fails loudly before any record is touched; parse_wire is hoisted above the loop so a bad mode cannot half-apply.

Known scope limits: records created via snapshot import or team import set mirrors at creation, so the heal does not fire for them however desynced; the healed population is pre-fix mints only.

Sibling PR: #4270 fixes the dialog's wire patch (defect 1). No file overlap — this PR is Rust src-tauri/commands/personas/, that one TypeScript features/agents/ui/. Neither carries a closing keyword; #2501 should be closed by hand when defect 1 lands.

Tests

• 16 pinned regression tests in update/behavior_cascade_tests.rs — inheritance, pins (explicit, same-mode-allowlist, reordered-allowlist, duplicate-principal, stamped-mirror), the empty-allowlist skip, parallelism-through-unsafe-state, the pre-fix heal and its negative, adopted-set retention scoping, and fail-loudly on a bad mode.
• Full desktop lib suite: 2104 passed, 0 failed.
• cargo clippy --workspace --all-targets -- -D warnings clean; pnpm typecheck clean.

Refs #2501

[georgerous]

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.

[iroiro147]

@georgerous both asks are in, pushed as 7acfea04:

1. Mint guard mirrored — a definition in allowlist + [] now skips adoption (mirrors still refresh) instead of cascading the crash-shape. Skip, not fail, as you asked: that state is reachable through the person-picker today and a hard error would wedge every other edit on the persona.
2. Allowlist joins the discriminant — a same-mode allowlist pin (the EditRespondToDialog workaround shape) no longer reads as inheriting. One subtlety your review surfaced indirectly: both the comparison AND the adoption now use the effective allowlist (empty for non-allowlist modes, matching apply_persona_behavior's asymmetry) — writing the definition's raw list on a cascade to anyone would have broken the next edit's inheritance detection the same way.

Also hoisted the wire parse above the loop, so a bogus mode can no longer half-apply mirror refreshes.

Three new tests pin all of it: empty_allowlist_definition_skips_adoption_without_failing, same_mode_allowlist_pin_survives_definition_edit, cascade_to_non_allowlist_mode_clears_stored_allowlist — 9/9 in the module.

On your scope note: agreed #2501 defect 1 (person-picker never persisting the allowlist) stays open regardless — I've already downgraded #4270 from Fixes to Refs so the issue can't auto-close over it. Your end-to-end matrix offer stands accepted; the #4115 skip is now in.

[xtranger51]

Independent verification + two observations. I hit exactly #2501 in production this week (Desktop v0.5.14, macOS Apple Silicon, self-hosted relay from ghcr.io/block/buzz:main, membership-required community): granting a second human member via the persona Advanced settings landed on definition_respond_to only, and the harness kept booting respond_to=owner-only across restarts. While preparing a fix I found this PR — it takes the same shape I arrived at (cascade in update_persona_with, mirroring propagate_persona_name_rename), and the inheriting-vs-pinned discriminant here is more careful than what I had, so I tested this PR instead of opening a duplicate.

Verification against current main (f956e6f):

• gh pr diff 4115 applies cleanly (no conflicts).
• Full desktop Tauri suite: 2449 passed, 0 failed (includes the 9 behavior_cascade_tests).
• cargo clippy --workspace --all-targets -- -D warnings: clean.

Two things I noticed reading the diff against my production store, for whatever they're worth:

1. Adopted records don't republish their retained kind:30177 projection. agent_event_content puts the behavioral triple (respond_to, respond_to_allowlist, parallelism) on the wire, so after a cascade the relay's retained agent record stays stale until the next boot reconcile. The display_name cascade retains for exactly this reason (the #2423 comment in update_persona_with). Suggest calling retain_managed_agent_pending for records the cascade actually adopted (mirror-only refreshes can skip it).

2. Already-desynced installs never heal through this cascade. Every instance minted before this fix, on a definition that was already edited during the buggy era, fails the inheriting test forever. Concrete bytes from my store: definition respond_to="allowlist", definition_respond_to_allowlist=["bc6a…"]; instance respond_to="owner-only", respond_to_allowlist=[] (mint-time value, mirrors absent). Any future edit computes old_mode=Allowlist, inherited_allowlist=["bc6a…"] — the instance matches neither, reads as an explicit pin, and is preserved. Given how many desynced stores the #2501 thread implies, maybe worth handling: instance mirror fields are only ever written by this PR's cascade, so definition_respond_to == None is a reliable "pre-fix record" marker. Something like: on cascade, treat a record with unset mirrors whose gate is still the mint default (owner-only + []) as inheriting. That heals the common desync without touching EditRespondToDialog pins (which are non-default by construction). Happy to test a follow-up commit against my repro either way.

(Investigation done with Claude Code against the live store + harness logs; test runs above are from a clean worktree.)

[iroiro147]

@xtranger51 Thank you — testing this instead of opening a duplicate was the generous call, and both observations are right. Both are implemented in c805bdf4.

1. Retained kind:30177 projection. Correct, and the precedent you point at is the right one. propagate_persona_behavior now reports which records actually adopted, and update_persona_with retains exactly those, mirroring the #2423 rule. I took your parenthetical seriously and scoped it: mirror-only refreshes do not retain, because the projection is unchanged and the call would be a guaranteed no-op. There is a test asserting a parallelism-only edit produces an empty adopted set.

2. Already-desynced installs. This is the more important of the two — without it the fix does nothing for the population that already has the bug, which the #2501 thread suggests is most of it. Implemented as you proposed: unset definition_respond_to marks a pre-fix record, and combined with a gate still at the mint default it is treated as inheritance. Your store bytes are the test fixture verbatim — definition allowlist + ["bc6a…"], instance owner-only + [], mirrors absent.

I also added the negative case, because the heal is the riskiest thing in this PR: a pre-fix record whose gate is not the mint default stays pinned. Your reasoning for why that is safe is what convinced me — before this fix the dialog could not write an instance-level gate at all (defect 1 of #2501), so a default gate on a pre-fix record cannot be a deliberate choice. Worth stating plainly as the residual risk: if some path I have not found could write owner-only + [] as a genuine pin, the heal would overwrite it. I could not find one.

One implementation trap worth recording, since it would be easy to repeat: the marker has to be read before the mirror refresh in the same loop iteration, because record.definition_respond_to = persona.respond_to.clone() runs first and overwrites it. Written the obvious way the heal silently never fires — no error, no failing behaviour, just a dead branch. My own test caught it, which is the only reason I noticed.

Verification: 15 behavior_cascade tests, full desktop lib suite 2103 passed / 0 failed, cargo clippy --all-targets -- -D warnings clean. One caveat I will not paper over: during this work a single run showed 1 failure I could not attribute before it cleared, and three consecutive full runs since have been clean. I did not capture the name, so I cannot tell you it was unrelated to this change — only that I could not reproduce it.

Also, following @georgerous's note about two PRs on one write path: this PR is now Refs #2501 rather than Fixes, so it will not auto-close the issue while #4270 (the wire-patch half) is still open. There is no file overlap between them — this one is Rust src-tauri, that one TypeScript features/agents/ui — so they cannot conflict in either merge order.

Your offer to test a follow-up against your repro is worth more than anything I can assert from here, and the heal is exactly the part that deserves it. If it misbehaves against a real desynced store I would rather hear it now than after a merge.

[georgerous]

@xtranger51 Flagging this here because it's about your observation 2, and I originally wrote it on #4270 where you may not have seen it. Real-store bytes beat anything either of us can assert from reading, so I'd value your take.

Your diagnosis is right and worth fixing — a pre-fix desynced instance does fail the inheriting test forever. My concern is the marker c805bdf4 uses to identify those records, not the goal.

The safety argument for pre_fix_default runs: unset mirrors ⟹ pre-fix record ⟹ a gate at owner-only + [] can't be a deliberate pin. The first step doesn't hold. create_managed_agent mints every instance with definition_respond_to: None (commands/agents.rs:831). So unset mirrors doesn't mean "record predates this work" — it means "no behavior cascade has touched this record yet", which is the state of every freshly minted instance today and after both PRs land.

Two consequences, opposite directions:

False positive, the one that worries me. #4270 (the other half of #2501) lands a dialog that reliably writes the instance-level gate, and its transition matrix includes allowlist → owner-only. So once both merge: mint an instance under an anyone persona, pin it to owner-only in the dialog, and you have unset mirrors + owner-only + [] — indistinguishable from your pre-fix shape. The next persona behavior edit silently un-pins it. That's the residual risk @iroiro147 stated and couldn't find a path for; the path is the sibling PR.

False negative, affecting your coverage estimate. Records created through commands/personas/snapshot/import.rs:652 and commands/team_snapshot.rs:609 set the mirror fields at creation, so the heal never fires for them however desynced they are. The healed population is narrower than "most of it". (migration/backfill.rs:107 also writes those fields, but onto a view_source clone rather than the stored record — I checked it before raising the other two; it isn't a counterexample.)

Your observation 1 and the retention scoping look unaffected by any of this, for what it's worth — excluding mirror-only refreshes from the retain is right.

Two things your store could settle that our reading can't:

1. On your desynced instance, is definition_respond_to actually absent in the stored bytes, or set to something stale? You reported "mirrors absent", which would mean the heal does fire for your shape — worth confirming against the file directly, since it's the fixture the new test is built on.
2. The falsification for the false positive, with both branches built: mint a fresh instance under a non-owner-only persona, pin it to owner-only through EditRespondToDialog, then make any behavior edit on the persona. If the pin survives, I'm wrong and I'll say so here.

The direction I suggested on #4270 was to make the marker mean what it's asserted to mean — stamp the mirror triple at mint in create_managed_agent, which has minted in hand — or move the heal to a one-shot boot backfill, where a live branch in the cascade can't catch future pins at all. Either way I'd rather it be settled against your store than my grep.