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.

[iroiro147]

@xtranger51 — your offer to test the heal against your real desynced store still open? @georgerous made the point on #4270 that a genuine pre-fix install is the one thing neither of us can synthesise, and I agree: my fixture is your reported bytes, which means it proves my code matches my reading of your report, not that either matches your disk.

One thing changed since you reported, and it changes what to look for. In c805bdf4 the heal identified pre-fix records by definition_respond_to.is_none(). @georgerous found that unsound — create_managed_agent minted every record with unset mirrors, so once #4270's dialog can pin a fresh instance to owner-only, the next persona edit would silently un-pin it. Fixed in 35cebd9e: mint now stamps the mirror triple from the linked definition, so unset mirrors means what the heal claims it means.

That makes your store the interesting case, because it cuts both ways:

1. Does the heal still fire for you? It requires definition_respond_to genuinely absent in the stored bytes plus a gate at owner-only + []. You reported "mirrors absent" — if your record instead has a stale value there, the heal skips it and your install stays desynced, which would mean the marker is wrong for real-world pre-fix records and I would want to know before this merges rather than after.
2. Does a fresh mint now stay pinned? Post-35cebd9e, minting an instance under a non-owner-only persona and pinning it to owner-only through the dialog should survive a persona behavior edit. That is the regression @georgerous found, and a real relay is a better test of it than my unit test.

Either result is useful — if (1) fails, the fix does not reach the population it was written for.

No pressure and no timeline; the PR is not blocked on it. If you would rather just paste the raw definition_respond_to / respond_to / respond_to_allowlist bytes for the desynced record and let me reason about it, that alone would settle (1).