Skip to content

fix(desktop): send complete respond_to wire patch on every allowlist transition - #4270

Open
iroiro147 wants to merge 1 commit into
block:mainfrom
iroiro147:fix/2501-respond-to-wire
Open

fix(desktop): send complete respond_to wire patch on every allowlist transition#4270
iroiro147 wants to merge 1 commit into
block:mainfrom
iroiro147:fix/2501-respond-to-wire

Conversation

@iroiro147

Copy link
Copy Markdown
Contributor

Summary

Fixes #2501.

The Desktop agent-instance edit dialog silently dropped the allowlist payload on two reachable mode transitions, so the buzz-acp harness kept starting with respond_to=owner-only regardless of what the UI showed. This made it impossible to share a custom agent with other community members through the UI — only a hand edit of managed-agents.json worked.

Root cause

The dialog gated the allowlist payload on submitMode === "allowlist" && listChanged, conflating "mode changed" with "payload changed":

  1. Mode flip → allowlist with an unchanged-but-preexisting list (e.g. anyone → allowlist where the list was populated in an earlier session): the mode flips on the wire but the harness never receives the full {mode, allowlist} tuple atomically, so the record stays in a state the harness reads as owner-only.
  2. The reporter also observed the visible desync — the UI wrote definition_respond_to (the definition snapshot projection) but the harness reads from the record-level respond_to + respond_to_allowlist the dialog never sent. (update_managed_agent merges undefined allowlist as "leave unchanged", so the two states stayed out of sync permanently.)

Fix

Extract a pure computeRespondToWirePatch helper in personaRuntimeModel.ts that owns the full contract:

  • respondTo on the wire only when the mode changed.
  • respondToAllowlist on the wire whenever submitMode === "allowlist" AND (modeChanged OR listChanged) — so a mode flip to allowlist always carries the complete {mode, allowlist} tuple the harness needs, even when the list itself happens to match.
  • Every other shape (flip away from allowlist, no-op, list edit under allowlist) produces exactly the minimal correct patch.

The dialog now spreads the helper's return into the submit input — there is one canonical wire-shape producer instead of inline conditional logic.

Reproduction (from the issue)

Under Buzz Desktop v0.4.23 (macOS, Apple Silicon), custom agent, runtime claude:

  1. Open agent config → "Who can talk to this agent" → Allowlist → add a member pubkey → Save. Works on first save.
  2. Re-open → switch to Anyone → Save → mode flips.
  3. Re-open → switch back to Allowlist → the list is already populated from step 1 → Save.
  4. Have the member @mention the agent — no response; harness log shows respond_to=owner-only.

Manual managed-agents.json edit of respond_to + respond_to_allowlist fixes the agent permanently — the file is the source of truth; the dialog simply wasn't writing it on transition 3.

Testing

  • New pure-module test desktop/src/features/agents/ui/agentInstanceEditRespondToWire.test.mjs — 6 tests covering the full transition matrix:
    • owner-only → allowlist with fresh list (mode + list)
    • allowlist → allowlist with edited list (list only)
    • allowlist → anyone (mode only)
    • owner-only → owner-only no-op (nothing)
    • anyone → allowlist with pre-existing list (mode + list, the previously broken path)
    • allowlist → owner-only (mode only)
  • Full webview suite: 3912 passing, 0 fail.
  • pnpm exec tsc --noEmit: clean.
  • The helper uses the shared RespondToMode type end-to-end so the wire shape is type-checked against UpdateManagedAgentInput.

Notes for reviewers

  • No schema or record migration — the fix is purely at the wire-shape producer layer; existing records self-heal on the next save-through-dialog.
  • The Rust update path is untouched and continues to enforce the allowlist-non-empty invariant; the UI's editValidity_allowlistWithEmptyList_blocksSave gate already prevents the crash-loop shape at the dialog.

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

…transition

The agent instance edit dialog gated the allowlist payload on
`submitMode === "allowlist" && listChanged`, silently dropping the payload
in two reachable shapes:

1. Mode flip to allowlist with a list that happens to match the persisted
   record (e.g. anyone → allowlist where the list was populated in an
   earlier session): the mode flips but the harness never receives the
   full `{mode, allowlist}` tuple atomically, so the agent stays
   unreachable to the intended members.
2. Mode flip to allowlist with a fresh list was covered only because the
   two gates happened to coincide.

The reporter (block#2501) observed the visible symptom as "UI edits don't
stick" — the UI wrote `definition_respond_to` (the snapshot projection)
but the harness reads from the record-level `respond_to` payload that the
dialog failed to send.

Extract a pure `computeRespondToWirePatch` helper in
`personaRuntimeModel.ts` that owns the contract end-to-end: send the
allowlist payload whenever the submitted mode is `allowlist` AND (mode
changed OR list changed); send the mode only when it changed. This is the
single canonical wire-shape producer; the dialog now spreads it into the
submit input. Six new pure-module tests pin each transition shape; the
behaviour is end-to-end exercised by `update_managed_agent`'s merge of
undefined mode/allowlist as "leave unchanged".

Fixes block#2501

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

Copy link
Copy Markdown

Read the diff — computeRespondToWirePatch is the right shape and the transition matrix is complete; the modeChanged || listChanged gate under submitMode === "allowlist" correctly re-sends the payload on the anyone → allowlist flip with an unchanged-but-preexisting list that was dropping it. Thanks for pulling it out as a pure, tested helper. Two things for reviewers before this carries Fixes #2501.

1. The "Rust path enforces the allowlist-non-empty invariant" note holds for this path, but not for the #4115 cascade path.

Your reviewer note is accurate for update_managed_agent, and the dialog's editValidity_allowlistWithEmptyList_blocksSave gate means computeRespondToWirePatch never actually emits allowlist + [] from here — the [...submitAllowlist] branch can only fire with a non-empty list because Save is blocked otherwise. Good.

But #4115 (your persona → instance cascade) reaches the record through a different Rust function, propagate_persona_behavior, whose inherit branch copies the definition allowlist verbatim with no non-empty guard:

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();

So a definition sitting in allowlist + [] cascades the exact crash-shape onto every inheriting instance — the state resolve_mint_behavioral_defaults / apply_persona_behavior reject at mint. Neither the dialog gate nor #4270 protects that path; only the mint-guard skip I asked for on #4115 does. If both PRs land, the invariant holds on the dialog write path but is still violable on the cascade write path — worth stating so #2501 isn't closed with that seam open.

2. Does this close MaxWynnDev's repro, or only the v0.4.23 transition-3 flow?

Your reproduction (v0.4.23, transition 3) is exactly what the helper fixes. But the other #2501 repro (MaxWynnDev, v0.5.2) is "the list never persists at all, even on first save" — and on first save this dialog already sent the allowlist under the old gate (submitMode === "allowlist" && listChanged, and [] → [A] is a list change), so #4270 changes nothing on that path. Either that repro hits a different write path (the persona person-picker / the profile panel #2505 touches) or a backend persist bug — worth confirming it's actually closed before this carries Fixes #2501, so we don't resolve the issue with the first-save case still live.

Happy to build #4270 + #4115 together and run the end-to-end matrix (fresh allowlist; anyone → allowlist with unchanged list; mixed inheriting/pinned fleet via a persona edit; restart + re-mint) once the #4115 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