Skip to content

fix(desktop-messages): preserve inline agent mentions with persistent addressing - #6793

Merged
tellaho merged 5 commits into
mainfrom
tho/restore-separate-agent-mentions
Aug 25, 2026
Merged

fix(desktop-messages): preserve inline agent mentions with persistent addressing#6793
tellaho merged 5 commits into
mainfrom
tho/restore-separate-agent-mentions

Conversation

@tellaho

@tellaho tellaho commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Category: fix
User Impact: Users can place @Agent mentions inline anywhere in a message—even when that agent is also persistently addressed—so separate agents can receive separate instructions in the same message.

Problem: Selecting an agent from the composer mention picker could turn the selection into persistent addressing instead of leaving an inline @Agent at the cursor. That made persistent addressing and inline composition mutually exclusive: once the persistent behavior took over, users lost the clear @Agent A do X, @Agent B do Y message structure they previously had.

Solution: Make inline mentioning and persistent addressing independent behaviors:

  • Ordinary mention selections always insert @Agent inline at the current cursor position.
  • This remains true when the agent is already persistently addressed; the persistent audience never blocks or replaces an inline mention.
  • If Automatically mention agents is enabled, a successfully sent inline mention may additionally make that agent persistent for later messages. Existing saved preferences remain respected.
  • The dedicated automatic-mention control and primary+Shift+Enter shortcut continue to add or remove persistent addressing directly.
  • For users without a saved preference, Automatically mention agents now defaults off.
File changes

desktop/src/features/messages/lib/autoPinMentionedAgentsPreference.test.mjs
Updates preference coverage for the default-off behavior while preserving explicit saved choices.

desktop/src/features/messages/lib/autoPinMentionedAgentsPreference.ts
Defaults automatic post-send persistence off when no valid preference exists.

desktop/src/features/messages/ui/useAgentAddressLockPicker.test.mjs
Verifies ordinary picker selections insert inline mentions without changing or pulsing the persistent audience, including when the selected agent is already persistently addressed.

desktop/src/features/messages/ui/useAgentAddressLockPicker.ts
Keeps ordinary mention selection on the existing inline insertion path while preserving the separate persistent-address controls and shortcut.

desktop/tests/e2e/persistent-agent-audience.spec.ts
Covers separately targeted inline instructions to two agents, signed outgoing recipients, default-off persistence, and explicit opt-in persistence scenarios.

Reproduction steps

Inline mentions without persistence

  1. Open a channel with at least two available agents and leave Automatically mention agents disabled.
  2. Use the composer mention picker to insert the first agent, type an instruction, then insert a second agent and type a different instruction.
  3. Confirm the draft reads like @Agent A review this, @Agent B test that and neither agent appears in the persistent addressed-agent controls.
  4. Send the message and confirm both agents are recipients while neither remains persistently addressed for the next draft.

Inline mentions with persistence

  1. Enable Automatically mention agents, then mention an agent inline and send successfully.
  2. Confirm that agent becomes persistently addressed for later messages.
  3. In a new draft, select the same agent from the mention picker again.
  4. Confirm a new inline @Agent is inserted at the cursor while the agent remains persistently addressed.
  5. Confirm the dedicated automatic-mention control and primary+Shift+Enter shortcut can still add or remove persistent addressing directly.

Demo

  • Before: Persistent addressing could consume an ordinary picker selection, preventing users from placing that agent inline in the message.
  • After: Ordinary selection always produces an inline @Agent; persistence is a separate optional behavior that can coexist with inline mentions.

Related issue

N/A — reported through the Buzz feature room.

Testing

At a4579c0a666f3644abe44271614186d7b9356bf5:

  • Post-push hooks passed desktop check, desktop typecheck, and the full desktop unit suite.
  • The two persistence-focused Playwright smoke scenarios passed after explicitly opting into Automatically mention agents.
  • The one-time multi-agent targeting and persistent-address shortcut Playwright scenarios passed on the feature changes before the final rebase; CI is validating the rebased branch.

@tellaho
tellaho force-pushed the tho/restore-separate-agent-mentions branch from ce944f5 to a4579c0 Compare August 25, 2026 17:22
@tellaho

tellaho commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

AI-assisted update: the default-off change left two persistence-focused smoke scenarios relying on the former implicit opt-in. Commit a4579c0 now enables Automatically mention agents through the real composer control before exercising post-send persistence, and restores the undone agent through the dedicated persistent-address action rather than ordinary inline selection. Verified with a fresh E2E build, Biome, both affected Playwright smoke scenarios (2/2 passed), and passing push-hook desktop check, typecheck, and full unit suite.

@tellaho tellaho changed the title fix(desktop-messages): restore one-time agent mentions fix(desktop-messages): preserve inline agent mentions with persistent addressing Aug 25, 2026
@tellaho
tellaho marked this pull request as ready for review August 25, 2026 17:24
@tellaho
tellaho requested a review from a team as a code owner August 25, 2026 17:24

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

The production path for one-time mentions looks correct at a4579c0a666f3644abe44271614186d7b9356bf5, but the updated smoke E2E contract is internally inconsistent with the new default and is failing locally.

In desktop/tests/e2e/persistent-agent-audience.spec.ts, the mention button opens settings and can undo an address removes Morgarita from the persistent audience, selects the ordinary Mention Morgarita action, sends, and then expects the persistent lock to return. With DEFAULT_KEEP_MENTIONED_AGENTS_PINNED = false, useAutoPinMentionedAgents exits without promotion, so the final expectation at lines 340-342 cannot hold unless this scenario explicitly enables automatic mention persistence. Either opt in with enableAutomaticMentionPersistence(composer) if repinning is the behavior under test, or keep the final lock count at zero to test the new one-time default.

Focused reproduction:

pnpm test:e2e:smoke tests/e2e/persistent-agent-audience.spec.ts --grep "the mention button opens settings and can undo an address"

Result: 1 failed. The run currently fails earlier at the post-removal empty-editor assertion because draft text is restored, so that clear/reopen sequence also needs to be stabilized before the final persistence assertion is exercised.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — REQUEST CHANGES on exact head a4579c0a666f3644abe44271614186d7b9356bf5.

Required fixes

1. Isolate the persisted opt-in in the persistent-audience E2E spec

desktop/tests/e2e/persistent-agent-audience.spec.ts enables buzz.messages.keepMentionedAgentsPinned in its opt-in journeys (enableAutomaticMentionPersistence, lines 34–45; callers at lines 431 and 481), but does not reset that localStorage key between tests. The reused Playwright worker storage therefore leaks "true" into later default-off cases. A complete targeted run produced 8 failed / 5 passed; default-off inline Tab/picker cases unexpectedly auto-pinned, and later opt-in cases found the toggle already checked.

Author action: add spec-level pre-load isolation (for example, test.beforeEach + page.addInitScript) that removes/resets buzz.messages.keepMentionedAgentsPinned. Seed "true" only for cases explicitly validating saved-preference/opt-in compatibility. Keep the behavior assertions strict.

2. Update required smoke journeys that still assert the superseded targeting contract

PR-relevant required smoke failures still expect ordinary agent selection to consume inline text and establish persistent addressing:

  • desktop/tests/e2e/mentions.spec.ts:1263
  • desktop/tests/e2e/mentions.spec.ts:2620
  • desktop/tests/e2e/persistent-agent-audience.spec.ts:263
  • desktop/tests/e2e/send-channel-binding.spec.ts:51

That conflicts with this PR’s intended contract: ordinary @Agent insertion remains inline, while persistent addressing is a separate explicit action. These are stale required-suite expectations rather than evidence that the new inline behavior itself is wrong, but required CI cannot pass with them unchanged.

Author action: update those journeys to use the dedicated persistent-address control/shortcut wherever persistence is intended. Preserve or add explicit coverage for ordinary inline mentions, targeting an already-addressed agent inline, default-off behavior, saved true opt-in, main/thread parity, cursor placement, and audience/channel binding.

After both fixes, rerun the complete targeted persistent-audience spec and all Desktop Smoke E2E shards. Diagnose any remaining pulse/shake/send-lifecycle failure independently rather than assuming it cascades from storage contamination.

Evidence and residual confidence

At this exact head:

  • cd desktop && pnpm test5,476/5,476 passed
  • desktop check/typecheck — passed; only unrelated existing Biome diagnostics
  • cd desktop && pnpm build:e2e && pnpm exec playwright test --project=smoke tests/e2e/persistent-agent-audience.spec.ts8 failed / 5 passed
  • GitHub required smoke shards 2 and 3 were confirmed failed while the workflow remained active; integration shards 1/2 passed

Static tracing found the intended recipient and lifecycle protections: normalized-pubkey merge/deduplication, persistent metadata limited to delivered recipients, post-success auto-pin, revision-guarded update/undo, explicit stored true/false preservation, and shared channel/thread persistent scope. No independent production-path defect was established in those flows.

Native Tauri interaction proof was not obtained. The attempted broad local smoke launch encountered launch-induced localStorage SecurityErrors and is not valid product evidence. This is a confidence gap, not an additional author-blocking defect. A native-capable verification owner should exercise default-off and saved-true opt-in, dedicated addressing plus ordinary inline mention, already-addressed targeting, cursor/shortcut/accessibility behavior, and main/thread parity after the required test fixes land.

@tellaho

tellaho commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

AI-assisted update: addressed the requested smoke-suite corrections in 70541f7272216de43180c879717f8b4c7a8e627f and 5623abbe0da7b117a526fd24be7a260bc5491274. The persistent-audience spec now clears its localStorage preference before every test; opt-in scenarios enable it deliberately. Stale journeys now use the dedicated Automatically mention action when testing persistent addressing, while the remove-then-inline flow correctly remains unpinned under the default-off UX. Verified against a fresh E2E build: the complete persistent-audience spec passed 13/13, both corrected mention journeys passed 2/2, and send-channel-binding passed 2/2. Biome, desktop check, and typecheck passed; the full desktop unit run reached 5,475/5,476 with the pre-existing Node loader ERR_INVALID_RETURN_PROPERTY_VALUE in useRetainedProjectGitViews.test.mjs.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

The preference isolation and updated one-time-versus-persistent expectations are correct, but the previously reported focused smoke failure remains at exact head 5623abbe0da7b117a526fd24be7a260bc5491274.

desktop/tests/e2e/persistent-agent-audience.spec.ts still clears the editor with input.fill(""), reopens the mention picker, and then observes the old draft text restored. The focused journey fails at the empty-editor assertion before it can exercise the corrected default-off lock assertion. Please stabilize that clear/reopen sequence and rerun the focused case.

Focused reproduction:

pnpm test:e2e:smoke tests/e2e/persistent-agent-audience.spec.ts --grep "the mention button opens settings and can undo an address"

Result at this head: 1 failed at persistent-agent-audience.spec.ts:329 with draft text still present.

@tellaho
tellaho force-pushed the tho/restore-separate-agent-mentions branch from 5623abb to c5ff644 Compare August 25, 2026 18:35
@tellaho

tellaho commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

AI-assisted update: the focused composer journey now waits for both the cleared editor and disabled send control before reopening the mention tray, removing the clear/reopen race that could restore draft text. The branch was also rebased onto current origin/main. Verified at c5ff644258016d00635b2f347974201f8e3a7e11: the focused journey passed 10/10 repeated runs, the complete persistent-audience smoke spec passed 13/13, Biome passed, and the push-hook desktop check, typecheck, full unit suite, file-size gate, and branch-skew gate all passed.\n\n_AI-generated by Carl._

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — REQUEST CHANGES

Reviewed: 113a33b7e49b7173ee1767c49ef2f49c63803034..5623abbe0da7b117a526fd24be7a260bc5491274 (exact head 5623abbe0da7b117a526fd24be7a260bc5491274)

Risk: high — this changes message addressing, draft state, and send lifecycle behavior.

Blocking finding

desktop/src/features/messages/ui/useAgentAddressLockPicker.ts:68-74 now removes the persistent audience entry without synchronizing the editor/draft state. In the existing “mention button opens settings and can undo an address” journey, the user types draft text, clears the composer, then removes the automatic address. The old draft text is resurrected. The repository assertion at desktop/tests/e2e/persistent-agent-audience.spec.ts:329 correctly expects the cleared composer to remain empty, but receives draft text every clean first run.

This can make a user send text they explicitly deleted after changing persistent addressing. It is also a PR-caused required smoke failure, even though the sharded CI run happened to go green.

Author action: keep the cleared draft authoritative when removing a persistent address, and retain a regression assertion that deleted text does not return. Then rerun the complete persistent-audience spec from a fresh E2E build. Please do not “fix” this by changing line 329 to expect the stale draft.

Verification owner: author patches; reviewer reruns the complete persistent-audience spec and affected smoke journeys at the replacement exact head.

Reconciled validation

  • pnpm lint: pass (pre-existing/unrelated Biome diagnostics only)
  • pnpm typecheck: pass
  • full Desktop unit suite: 5,476/5,476 pass
  • changed mention, channel-binding, keyboard/cursor, lifecycle, and visual placement checks passed in the product/UI lane; generated light/dark/narrow screenshots showed no clipping or overlap.
  • Fresh pnpm build:e2e, then the two explicit-address journeys plus two channel-binding journeys: 4/4 pass at matching clean HEAD. This disproves an earlier stale-build signal that the dedicated toggle left an inline chip.
  • Fresh pnpm build:e2e, then complete persistent-agent-audience.spec.ts: 1 failed, 12 passed. Failure is line 329: expected empty, received draft text.
  • Repeating that journey reproduced the stale-draft failure on the next clean first run; later repeats also exposed the known local preview-server death, which is not being promoted as a product finding.
  • GitHub required checks were green and live head remained exact immediately before submission. Green shards do not override the locally reproduced required smoke failure.

Manual/native evidence: browser Playwright screenshots were inspected; no native Tauri lifecycle capture was obtained.

Residual risk: native focus, screen-reader announcement, and platform keyboard behavior remain unwitnessed. Static tracing found no separate recipient signing, deduplication, send/retry, or channel/thread audience-state defect.

@jedwards27
jedwards27 dismissed their stale review August 25, 2026 18:36

Dismissed immediately: the PR head moved from reviewed 5623abb to c5ff644 before submission. This review must not apply to the unreviewed head.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — REQUEST CHANGES

Reviewed: 113a33b7e49b7173ee1767c49ef2f49c63803034..c5ff644258016d00635b2f347974201f8e3a7e11 (exact head c5ff644258016d00635b2f347974201f8e3a7e11)

Risk: high — this changes message addressing, editor/draft state, and send lifecycle behavior.

Blocking finding

desktop/tests/e2e/persistent-agent-audience.spec.ts:317 exposes an unresolved production race: after the journey preserves draft text, the user explicitly clears the composer with input.fill(""), but stale editor/draft synchronization can restore draft text. The new head adds assertions for empty input and disabled send, but contains no production fix.

Both independent lanes reproduced this from fresh E2E builds. One complete spec run failed only here (1 failed, 12 passed) while its dedicated server remained alive. A separate focused --repeat-each=3 run produced 2 failures, 1 pass, both receiving draft text at line 317 after the five-second timeout. A newer explicit clear must win; otherwise a user can send text they deliberately deleted.

Author action: fix the production editor/draft synchronization so an explicit newer clear wins over stale settlement/rerender. Keep the empty-input and disabled-send assertions, and add deterministic controlled-timing coverage for preserve draft → clear → async settlement/rerender → remains empty.

Verification owner: author patches; reviewer reruns the focused race repeatedly and the complete persistent-audience spec at the replacement exact head.

Reconciled validation

  • Complete persistent-agent-audience.spec.ts: 1 failed, 12 passed from a fresh build; sole product failure at line 317.
  • Focused line-317 journey once: pass; --repeat-each=3: 2 failed, 1 passed, confirming nondeterminism rather than a stable test expectation.
  • Explicit-address channel + DM journeys and both send-channel-binding journeys: 4/4 passed from a fresh build. The earlier expired-head inline-chip diagnosis does not reproduce and is retracted.
  • Prior exact-head evidence also had pnpm lint pass (unrelated existing diagnostics only), pnpm typecheck pass, full Desktop unit suite 5,476/5,476 pass, plus keyboard/cursor/lifecycle and generated light/dark/narrow visual checks passing.
  • Static tracing found no separate recipient signing, deduplication, send/retry, or channel/thread audience-state defect.

Manual/native evidence: browser Playwright screenshots were inspected on the prior exact head; no native Tauri lifecycle capture was obtained.

Residual risk: native focus, screen-reader announcement, and platform keyboard behavior remain unwitnessed. CI was still running at lane completion; regardless, a green shard cannot override this independently reproduced required smoke failure.

tellaho and others added 4 commits August 25, 2026 12:04
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
@tellaho
tellaho force-pushed the tho/restore-separate-agent-mentions branch from c5ff644 to c6b5333 Compare August 25, 2026 19:16
@tellaho

tellaho commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

AI-generated update: The blocker was a race where an explicit composer clear could be overwritten by stale draft/editor synchronization after an async rerender or cleanup. The fix now tracks authored draft content, treats an explicit empty edit as authoritative, excludes restoration callbacks from authored edits, and keeps the live content ref synchronized with editor updates. I also rebased over #6714 without replacing its explicit persistent-address shortcut/action behavior; persistence-specific E2E scenarios now opt in, while ordinary Tab-selected mentions assert the default one-time behavior.\n\nValidation at c6b53334e9d676878295670d7357314b51fb1f85: focused unit tests 25/25; complete persistent-agent-audience.spec.ts 16/16; mention and send-channel-binding smoke journeys 75/75; E2E build/typecheck and Biome passed. The pre-push full desktop unit lane reported one unrelated loader failure in useRetainedProjectGitViews.test.mjs; rerunning that exact test passed 1/1, after which the already-validated branch was pushed without rerunning the full hook.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — REQUEST CHANGES at exact head c6b53334e9d676878295670d7357314b51fb1f85 (base f24971033178926153b49d320bd876d15d9cb2bf).

The prior plain-text explicit-clear race is fixed within one workspace, but the replacement module-global clear tombstone introduces two deterministic unsent-draft data-loss paths.

1. High — clearing caption text discards a retained attachment on remount

desktop/src/features/messages/ui/useDraftPersistSnapshot.ts:150-176 treats a tombstoned key as though the complete saved draft is absent, clearing body text and pending imeta/spoiler state. But trackAuthoredContent("") at :200-217 persists still-present imeta, and desktop/src/features/messages/lib/useDrafts.ts:426-456 deliberately supports attachment-only drafts.

A production-hook probe mounted a persisted caption + image, cleared only the caption, then remounted. The store retained one image immediately after the clear, but remount restored zero:

REVIEW_PROBE_explicit_text_clear_preserves_remaining_image_on_remount
AssertionError: remaining image must restore after remount
0 !== 1

Consequence: attach an image, type and delete its caption, then switch channel/thread or otherwise remount the composer; the unsent image disappears despite never being removed. Main and thread composers share this hook.

Author action: make empty body content authoritative without treating the whole saved draft as absent. Restore retained attachment, queued-file, and spoiler metadata while fencing only stale text. Add a production-hook regression for caption + image → clear caption → remount → empty body and image retained, with queued-file parity.

Verification owner: author patches; reviewer reruns the new regression, complete Desktop unit suite, and complete persistent-audience suite at the replacement head.

2. High — clear tombstones cross identity/community draft stores

desktop/src/features/messages/ui/useDraftPersistSnapshot.ts:72 creates a process-global Set<string>. Clear and restore operations key it only by effectiveDraftKey (:150-157, :200-209). Draft storage, however, is intentionally scoped by relay + pubkey (desktop/src/features/messages/lib/useDrafts.ts:105-137), and workspace reinitialization does not reset this parallel tombstone set.

Two independent production-hook probes reproduced the same failure: identity/workspace A clears key X; workspace B has a private saved draft at the same key X; mounting B suppresses its draft and can persist the resulting empty state. One deterministic result was:

expected: "B private draft"
actual:   ""

Consequence: switching identities or communities can silently hide and then delete another workspace's valid unsent draft when channel/thread IDs collide.

Author action: remove the unscoped global marker, or scope/reset clear authority atomically with the same relay + pubkey identity as the draft store. Prefer keeping authoritative content revision/fencing in the owning draft lifecycle rather than a parallel global set. Add A clear → switch relay/identity → B same-key draft restores unchanged, then switch back to A to prove stale text does not resurrect.

Verification owner: author patches; reviewer reruns the cross-workspace regression, repeated focused explicit-clear race, main/thread parity, complete persistent-audience spec, and Desktop package gates at the replacement head.

Exact-head evidence

  • Focused explicit-clear journey with --repeat-each=10 --workers=1: 10/10 pass.
  • Complete persistent-agent-audience.spec.ts: 16/16 pass.
  • Full just desktop-test: 5,498/5,498 pass.
  • E2E build, typecheck, and check: exit 0; check reported only pre-existing unrelated diagnostics.
  • Both adversarial production-hook probes failed as described; probe edits were reverted and both reporting worktrees were clean at exact head.
  • Live PR head was reverified as c6b53334e9d676878295670d7357314b51fb1f85 immediately before review submission. Required Desktop Core, smoke, macOS build, and integration jobs were still in progress; this verdict rests on the concrete reproduced defects, not pending CI.

Confidence gap: native Tauri focus, OS keyboard, and screen-reader behavior were not exercised. No separate cursor/keyboard/accessibility defect was established; native observation remains reviewer-owned after the blockers are fixed.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: f24971033178926153b49d320bd876d15d9cb2bf..c6b53334e9d676878295670d7357314b51fb1f85 (exact head c6b53334e9d676878295670d7357314b51fb1f85)
Risk: high — the change repairs draft synchronization but introduces unsent-draft/attachment loss across remount and workspace boundaries.

Behavior/contracts traced: composer authored-content fencing; persisted body, imeta/queued-file, and spoiler restoration; main/thread composer lifecycle; relay+identity draft-store scoping; explicit clear and remount behavior.

Findings — blocking:

  1. desktop/src/features/messages/ui/useDraftPersistSnapshot.ts:150-176,200-217 — the authoritative-clear path treats a cleared body as if the entire saved draft were absent. An attachment-only draft is valid, but after clearing only a caption and remounting, restoration clears pending imeta/spoiler state too. A production-hook probe preserved one image in the store immediately after the text clear, then restored zero images after remount. Users can lose an unsent attachment merely by deleting its caption and switching channel/thread.

    Author action: fence stale body text without converting the whole draft to absent. Preserve saved attachment/queued-file/spoiler metadata, and add production-hook regressions for caption + image → clear caption → remount (empty body, image retained), including queued-file parity.

  2. desktop/src/features/messages/ui/useDraftPersistSnapshot.ts:72,150-157,204-208authoritativelyClearedDraftKeys is process-global and keyed only by effectiveDraftKey, while desktop/src/features/messages/lib/useDrafts.ts:105-137 scopes persisted drafts by relay + identity. Clearing key X in workspace A suppresses a valid draft at the same key in workspace B after an identity/community switch; the second draft can then be cleared during cleanup. Two independent production-hook probes reproduced expected workspace-B content becoming "".

    Author action: scope/reset clear authority with the same relay+pubkey ownership as the draft store, or keep the authoritative revision inside the owning draft lifecycle. Add A clear → switch relay/identity → B same-key draft restores unchanged, then switch back to A and prove stale text does not resurrect.

Verification owner: author patches; reviewer reruns both deterministic production-hook regressions, focused explicit-clear repetition, complete persistent-audience spec, and full Desktop package gates at the replacement exact head.

Validation at exact clean head:

  • focused explicit-clear journey, --repeat-each=10 --workers=1: 10/10 passed
  • complete persistent-agent-audience.spec.ts: 16/16 passed
  • full Desktop unit suite: 5,498/5,498 passed
  • E2E build, typecheck, and check: exit 0 (unrelated existing diagnostics only)
  • adversarial production-hook probes: failed deterministically as described; probe edits reverted and checkout reverified clean

Manual/native evidence: no native Tauri focus, platform-keyboard, or screen-reader run. This is a reviewer-owned confidence gap, not an additional author blocker.

Residual risk: native focus/accessibility behavior remains unwitnessed; required CI jobs were still completing at the final evidence poll. Neither gap changes the two reproduced data-loss defects above.

Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@buzz.block.builderlab.xyz>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
@tellaho

tellaho commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

AI-generated update: The blockers were two draft data-loss boundaries in the explicit-clear fence. The clear authority now suppresses only stale body text and mention routing while still restoring persisted image/spoiler metadata and retained queued files; its key also includes the draft store’s relay + identity scope so one workspace cannot suppress another workspace’s same-key draft. Added production-hook regressions for caption + image/spoiler, caption + queued file, and A-clear → workspace-B restore → A-remount.\n\nValidation at 55bf9a0a16e46c659d652a16cd105f434b2f6107: focused draft/store tests 56/56; complete persistent-audience smoke spec 16/16; Desktop check, typecheck, and E2E build passed. A focused smoke repetition had one unrelated fixture-start timeout before the composer ingress mounted (9/10); the complete spec then passed. The pre-push Desktop unit lane reached 5,500/5,501 with the known Node loader failure in useRetainedProjectGitViews.test.mjs; that exact test passed 1/1 alone, so the validated commit was pushed without rerunning the full hook.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — APPROVE at exact head 55bf9a0a16e46c659d652a16cd105f434b2f6107 (base f24971033178926153b49d320bd876d15d9cb2bf).

No author-actionable defect remains in the reviewed systems/integration or product/UI paths. The two prior data-loss blockers are resolved.

Why the fixes hold

  • desktop/src/features/messages/ui/useDraftPersistSnapshot.ts:155-176 now uses clear authority only to restore empty body content and mention refs; it still restores saved imeta and spoiler state. Cleanup at :187-203 persists authoritative empty text with retained attachments, spoilers, and queued files.
  • Clear authority is now keyed through getDraftStoreScope() (useDraftPersistSnapshot.ts:73-77,155-160,208-225). desktop/src/features/messages/lib/useDrafts.ts:109-123,131-140 derives that scope from the same relay + pubkey storage bucket as the draft. The A-clear → B same-key restore → A-remount regression passes without resurrecting A's stale text.
  • Main and thread composers share this MessageComposer lifecycle path. Ordinary selection still inserts inline at the live cursor, while the automatic persistent-address toggle remains separate (useAgentAddressLockPicker.ts:185-290). Default-off preference preserves explicit saved true/false values.
  • Existing autocomplete-first keyboard routing, separate explicit shortcut, control semantics, live announcements, and name-collision identity disclosure remain intact (MessageComposer.tsx:705-766,923; MentionAutocomplete.tsx:142-149,180-223,291-410).

Exact-head validation

At matching clean head:

  • New production-hook lifecycle file: 11/11 pass.
  • Focused lifecycle/preference/addressing units: 36/36 pass.
  • Full Desktop unit suite: 5,501/5,501 pass.
  • Complete persistent-agent-audience.spec.ts --workers=1: 16/16 pass in the valid run, covering queued upload lock, explicit clear, default-off inline selection, opt-in persistence, and channel→thread synchronization.
  • Desktop check and typecheck: exit 0; check emitted four pre-existing unrelated diagnostics.
  • E2E TypeScript compile and Vite E2E build: pass.
  • git diff --check: pass; reporting worktrees were clean.
  • Mutation checks were causal: restoring whole-draft suppression broke image/spoiler restoration; reverting scoped authority to plain draft keys broke workspace-B restoration. The queued-file row also passed against the old hook because that queue is independently retained, so it is parity evidence rather than causal evidence for this delta.

A second Playwright attempt lost its local preview server after three passes and produced 13 ERR_CONNECTION_REFUSED failures; that is excluded from product evidence, not attributed to the PR.

Author action: none.

Residual confidence / verification owner: native Tauri focus, OS keyboard delivery, and screen-reader output were not exercised under shared-machine GUI safety constraints. A native-capable reviewer owns that optional observation. Exact-head required Desktop Core, smoke, macOS build, and integration CI jobs were still running at submission with no reported failure; branch protection/CI owns those gates. Any new head invalidates this approval.

@tellaho
tellaho dismissed wesbillman’s stale review August 25, 2026 20:32

feedback addressed at 55bf9a0

@tellaho
tellaho merged commit b58de7c into main Aug 25, 2026
24 checks passed
@tellaho
tellaho deleted the tho/restore-separate-agent-mentions branch August 25, 2026 20:32
wpfleger96 pushed a commit that referenced this pull request Aug 25, 2026
…picker

* origin/main:
  docs(nest): make commit attribution policy-neutral (#6707)
  fix(desktop-messages): preserve inline agent mentions with persistent addressing (#6793)

Signed-off-by: Hayt <9e1c23a3fd83f61da34420e4e88ff1b16e45cafcc0cd9019eb07d4ecfa8ca9b0@buzz.block.builderlab.xyz>
wpfleger96 pushed a commit that referenced this pull request Aug 25, 2026
* origin/main:
  docs(nest): make commit attribution policy-neutral (#6707)
  fix(desktop-messages): preserve inline agent mentions with persistent addressing (#6793)
  Qualify canonical relay images for staged delivery (#6781)
  feat(desktop): persist agent addressing across composer messages (#6714)
  feat: navigate images across message threads (#6705)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
wpfleger96 added a commit that referenced this pull request Aug 25, 2026
Main #6793 grew MessageComposer.tsx to the 1000-line ceiling; the branch's
gifMediaController prop tipped it one line over the ratchet. Drop the
redundant trackDraftAuthoredContent destructure alias (single call site) to
reclaim the line without touching the ratchet config or behavior.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96 pushed a commit that referenced this pull request Aug 25, 2026
…r-contracts

* origin/main:
  Remove public relay signing key fallback (#6729)
  docs(nest): make commit attribution policy-neutral (#6707)
  fix(desktop-messages): preserve inline agent mentions with persistent addressing (#6793)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
salman1993 added a commit that referenced this pull request Aug 25, 2026
…-layers

* origin/main:
  chore(release): release Buzz Desktop version 0.5.19 (#6828)
  Remove public relay signing key fallback (#6729)
  docs(nest): make commit attribution policy-neutral (#6707)
  fix(desktop-messages): preserve inline agent mentions with persistent addressing (#6793)

Signed-off-by: Salman Mohammed <smohammed@squareup.com>
wpfleger96 pushed a commit that referenced this pull request Aug 25, 2026
…-history

* origin/main:
  feat(desktop): add KLIPY GIF search to composers (#5554)
  fix(desktop): respect automatic mention preference after send (#6837)
  fix(release): attribute desktop candidates to the operator (#6831)
  fix(ci): check out source in docker.yml merge job (#6833)
  chore(release): release Buzz Desktop version 0.5.19 (#6828)
  Remove public relay signing key fallback (#6729)
  docs(nest): make commit attribution policy-neutral (#6707)
  fix(desktop-messages): preserve inline agent mentions with persistent addressing (#6793)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
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.

3 participants