fix(desktop): enforce agent mention authorization at send boundaries

[wesbillman]

Summary

• allow channel-member remote/headless agents only with current kind 10100 directory evidence, while stale member identities remain hidden
• fail closed while managed/relay directories load, error, or background-refetch across channel, forum, and cached autocomplete surfaces
• revalidate agent mention authorization immediately before normal sends and message-edit saves, including after deferred uploads
• in owner-only builds, fetch fresh authoritative profile ownership at send time and deny missing, changed-owner, or unavailable proofs
• preserve human mention tags when agent authorization is revoked or unknown

Supersedes #5536 because its contributor-fork head cannot be updated by maintainers.

Validation

Exact head: 7278cdd5fbcee676c7b858ea098503c62eeeff0d

• mandatory pre-push suites passed: desktop check/typecheck/tests, Rust tests, mobile tests, desktop Tauri checks, branch-skew
• desktop unit tests: 4,732 passed
• focused edit/ownership regressions: 8 passed
• focused mention E2E: 5 passed (remote positive, stale-member negative, directory error, pre-send revocation, mid-send revocation)
• file-size ratchet passed

One first focused E2E batch had a timing-only miss where the send click did not emit; the isolated rerun passed. One separate pre-push attempt hit the existing randomized passphrase separator test; the successful exact-head push reran and passed the mandatory suite.

[wpfleger96]

🤖 Combined review — two independent agent passes (Paul + Thufir), synthesized. Not approving yet: one authorization gap that both the verification trace and a full-suite run confirm should close before merge. Smaller items are inline.

Forum sends bypass the new revalidation boundary

desktop/src/features/forum/ui/ForumComposer.tsx:230-253 (file not in this diff, so noting here instead of inline): submitMessage calls the changed mentions.extractMentionPubkeys(trimmed) and passes that snapshot directly to submitter → ForumView.onSubmit → createPostMutation, which emits the p tags. It never calls the new mentions.revalidateMentionPubkeys, so forum posts/replies get only render-time filtering — a directory/allowlist/owner change between selecting an agent and submitting is not freshly checked, and the stale pubkey can still wake an agent current policy no longer authorizes. This is the same boundary this PR closes for chat sends and edit saves; the forum composer is a production consumer of the same mention API that misses it.

Suggested fix: await mentions.revalidateMentionPubkeys(mentions.extractMentionPubkeys(trimmed)) in ForumComposer.submitMessage before clearing/submitting, plus a regression that selects an agent, revokes authorization, submits, and asserts the signed forum event carries no agent p tag (the current forum e2e proves autocomplete visibility only, not emitted tags).

CI attribution (from the logs, not assumed)

The red checks are infra + pre-existing flake, not this diff: Desktop E2E Integration (2/2) died on docker compose (digest-mismatch, exit 101) before any test ran; Desktop Smoke E2E (4)'s failure is video-attachment.spec.ts:1242 with a virtualization.spec.ts flake — neither file is touched here; main's latest completed CI run failed the same Integration shards. The PR's own 12 new/changed mentions.spec.ts tests ran green in Smoke shard 3, and the full desktop unit suite (4,732) passes at this head.

What's solid

The tri-state admission model is the right shape. Chat sends and edit saves revalidate after deferred uploads and before signing/saving, owner-only mode fetches fresh verified ownership at send time, and error/missing/changed-owner all fail closed with unit coverage for each branch. Human mention tags survive revocation exactly as described. Most importantly the fix is tested at its seam: the e2e bridge now reads the signed event's actual p tags (pre-send and mid-send revocation), and deleting the production wiring turns the suite red — verified by mutation, not assumed from green.

[wpfleger96]

🤖 ownerOnly = false here swallows the undefined that useMentions passes while the owner-policy query is still loading (ownerOnly: agentAccessOwnerOnlyQuery.data), so the "unknown" branch getAgentMentionAdmission has for exactly that state is unreachable through this wrapper. During the window where the agent directories are ready but the owner policy isn't, an other-owned relay agent is admitted into autocomplete in an owner-only build. Send-time revalidation strips it on the chat/edit paths (it checks ownerOnly === undefined explicitly), which keeps this non-blocking — but it contradicts the fail-closed intent, and both reviews flagged it independently. Fix: drop the destructuring default and pass undefined through, or fold ownerPolicyReady into the directoryReady value this call site receives.

[wpfleger96]

🤖 This Set is rebuilt every render (on main, knownAgentPubkeys aliased the memoized mentionableAgentPubkeys) and sits in the dep arrays of handleMentionSelect and isAgentPubkey, so both recreate every render and the churn propagates into useMentionSendFlow's callbacks. Same pattern in useAgentMentionRevalidation: getSelectedAgentPubkeys: () => selectedAgentMentionPubkeysRef.current is a fresh arrow per render listed in its useCallback deps, so revalidateMentionPubkeys never holds identity and MessageComposer's submitMessage churns with it. Correctness is unaffected — this is render hygiene on the hottest composer path. Wrap the Set in useMemo; pass the ref (or a stable callback) for the getter.

[wpfleger96]

🤖 clearMentions() clears both name states and both maps but never resets this set, so it accumulates for the hook lifetime. The effect is conservative (extra pubkeys get revalidated, never skipped), so no correctness issue — but reset it in clearMentions for symmetry. Both reviews flagged this independently.

[wesbillman]

Reviewing on Wes's behalf at exact head 1709de5aec3fb77209a3b3895aa962c2edbf4635.

The forum authorization bypass is closed, but the repair introduces a blocking draft-loss race.

[P1] Forum edits made during final revalidation are silently discarded. ForumComposer.submitMessage snapshots trimmed and currentPendingImeta before awaiting mentions.revalidateMentionPubkeys (desktop/src/features/forum/ui/ForumComposer.tsx:218-236). That refresh performs relay directory/profile I/O. During the await, the editor and toolbar remain enabled: isSubmissionPendingRef guards only another submit, while richText is still editable and sendDisabled/toolbar state do not include the pending flag (:113-131, :380-386, :514-583). If the user types or adds content while the refresh is in flight, success builds/sends the old snapshot and then clears the current editor at :238-257, deleting the newer input without sending or restoring it.

Please either lock the forum editor/attachment controls for the full pending interval using rendered state, or re-read and atomically snapshot the current draft after revalidation before clearing. Add a deferred-revalidation regression: start submit, edit the draft while refresh is pending, release refresh, and prove the later input is neither silently cleared nor omitted.

The final forum gate itself is correctly placed before signing, preserves the draft on refresh/submission failure, and has signed-event coverage for revoked-agent tag removal. Chat and edit boundaries remain sound. The earlier owner-policy default and callback/set churn comments are worthwhile follow-ups, but send-time admission remains fail-closed, so they are not additional merge blockers.

[wesbillman]

Blocker: the new forum preflight can discard edits made while authorization refresh is in flight.

submitMessage snapshots trimmed and currentPendingImeta before awaiting revalidateMentionPubkeys, but it does not disable the editor/media controls during that await. After the await, it snapshots savedContent from the current editor and then unconditionally clears the composer, while finalContent is still built from the old pre-await trimmed value. If the user types or attaches media during a slow directory/profile refresh, a successful submission sends the old body and clears the newer body/attachment. The newer draft is restored only when submitter rejects.

This is especially plausible here because the preflight does two directory refetches and, in owner-only mode, a fresh profile fetch. The ref prevents duplicate submits, but it does not prevent editing, and it is not represented in the toolbar/editor disabled props.

Please make the snapshot/clear boundary coherent: either capture and clear the entire draft before the await and restore it on refresh/submission failure (while preserving any subsequently started draft), or explicitly disable editing/media for the whole pending interval and snapshot all fields consistently. Add a regression with a deferred revalidation promise: submit body A, edit to body B while deferred, release it, and prove B is neither cleared nor silently replaced by A.

The authorization direction is otherwise strong: current-head forum sends now share the final revalidation boundary, chat and edit paths revalidate after deferred uploads, human mention tags survive agent revocation, and owner-only proofs fail closed.

[wesbillman]

Release-safety follow-up reviewed by Carl on Wes’s behalf at exact head ae71c72c00af66e582f3c22747a77de7c3021c27: APPROVED.

I re-traced the complete authorization boundary across autocomplete, chat send (including deferred upload), edit save, and forum post/reply. The two prior forum blockers are closed: forum sends now revalidate before signing, and the composer plus media/overlay mutation surfaces are locked for the full asynchronous preflight so the submitted snapshot cannot discard concurrent edits. The regression verifies both editor and attachment immutability and asserts the signed outgoing event omits the revoked agent while retaining the attachment.

The policy remains fail-closed on directory loading/error/refetch, unknown owner policy, stale directory membership, and missing/changed/unavailable owner proof. Human mention tags are preserved. I found no remaining release blocker or compatibility/migration risk. GitHub CI is fully green on this exact head, including Desktop Core, all smoke shards, integration shards, builds, security, DCO, and cross-platform checks.

[wesbillman]

Correction to my release verdict: do not merge this head yet. Princess Donut identified a real P1 gap I missed.

At useMentionSendFlow.ts:401-475, completeSend classifies mentions from stale render-time state and can prepare a DM/channel, start or attach managed agents, and enroll agents into the active Huddle. Fresh authorization does not occur until finishSend at :556-568, after those side effects. A revoked/unknown agent can therefore be awakened or enrolled even though its final signed p tag is correctly stripped.

Required fix: revalidate before deriving any agent-preparation list and use only admitted pubkeys for channel preparation, start/attach, and Huddle enrollment; retain the existing final post-upload revalidation before signing. Add a regression asserting revocation causes zero preparation/enrollment side effects as well as no outgoing agent tag.

I previously focused the release pass on signed-event enforcement and the repaired forum mutation race, and failed to trace authorization backward through all pre-sign side effects. That was my miss, not an ambiguity in the code.

[wpfleger96]

🤖 Re-reviewed at head 939bf536 (two independent passes, Paul + Thufir, synthesized).

All prior feedback is addressed:

• Forum send bypass (blocking) — ForumComposer.submitMessage now awaits revalidateMentionPubkeys before building/submitting, keeps the draft on refresh failure, guards duplicate submits, and locks the composer surface (inert, disabled toolbar/attachments, guarded insert callbacks) while revalidation is in flight so the submitted snapshot can't race edits. Signed-event regression covers revocation mid-preflight, including proving a queued attachment can't be removed.
• ownerOnly = false fail-open window — the destructuring default is gone; ownerOnly is now required boolean | undefined, so an unresolved owner-policy query reaches the tri-state "unknown" branch and fails closed, with a unit test for exactly that state.
• clearMentions asymmetry — selectedAgentMentionPubkeysRef is now cleared.
• Memoization churn — knownAgentPubkeys is useMemo'd and the revalidation getter is a stable ref, restoring the identity chain.

Beyond the asked fixes, the new commits also close the invite/preparation side doors: revalidation now runs before channel prep, managed-agent start/attach, and huddle sync in completeSend, and again on Invite click in handleInviteNonMembers — with e2e coverage asserting a revoked agent produces neither a p tag nor any membership/lifecycle/huddle commands. Traced all revalidation call sites at this head; no remaining consumer of the mention API skips the boundary.

CI: the only red (Smoke E2E shard 3, messaging.spec.ts:1311 compact link preview geometry) fails identically on main's latest run (45f4b91a, the commit that introduced that test) and neither the test nor link-preview-attachment.tsx is touched by this branch — pre-existing main breakage, not this PR.