fix(desktop): show whether you can mention or invite an agent - #7239
fix(desktop): show whether you can mention or invite an agent#7239loganj wants to merge 8 commits into
Conversation
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
eec6f58 to
8478cfa
Compare
d0b8a9e to
6b81a62
Compare
8478cfa to
3d9452a
Compare
6b81a62 to
3d1a947
Compare
|
🤖 Stable-picker rewrite — integrated captureCaptured from exact integrated Live actions on stable rowsInvite is enabled; denied and unresolved known members are disabled Unavailable/Checking. The workflow subsequently checks five-second expiry and explicit Retry without identity/order movement. |
b0a4ecd to
1c9e2b0
Compare
30795ba to
6372d0e
Compare
1c9e2b0 to
5e10b5d
Compare
6372d0e to
f8bbe95
Compare
5e10b5d to
f10578c
Compare
0c8a241 to
cbde1a3
Compare
f8bbe95 to
58b41ae
Compare
1f82f67 to
bd20d08
Compare
fa638cb to
4c9aaa8
Compare
bd20d08 to
967a7a1
Compare
4c9aaa8 to
b354256
Compare
967a7a1 to
d53bc6a
Compare
b354256 to
b326af3
Compare
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
d53bc6a to
a6fb55d
Compare
b326af3 to
0464384
Compare
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
Keep the outgoing trigger input separate from the new message textarea, sample match controls together after inspector settlement, and verify the persisted original trigger and step text before reopen. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
The first send parks the pointer over the bottom-right error toast, which overlaps the send button; Sonner pauses its dismiss timer while the toaster is hovered, so the native retry click is intercepted for the full timeout. Move the cursor away and wait for the transient toast to clear naturally before retrying, mirroring the adjacent agent-startup-failure test. The injected first-send failure and every destination assertion are unchanged. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
The explicit-picker setup in the relay-agent directory-errors spec
cleared the seeded draft with fill(""), whose programmatic selectAll can
lose the selection to ProseMirror's own selection sync and leave
"@quinn" behind in CI, exhausting retries at the picker-empty
assertion. Clear the draft with native ControlOrMeta+A and Backspace so
the editor applies both steps in its own transaction, and gate
immediately on toBeEmpty before opening the picker. Every original
directory-error, disabled-row, Tab-completion and retry-recovery
assertion is unchanged; no production code, timeout or mock change.
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Logan Johnson <loganj@squareup.com>
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — REQUEST CHANGES at exact head f1af1b88c6458d40080529e8d71a0bdec1868a78 (base 0464384b676b7457f51876484e9beac998b690d5).
The full A Team review found three author-actionable defects:
-
DM rows promise an impossible
Invite…action.desktop/src/features/messages/lib/buildMentionCandidates.ts:264-273assigns every allowed nonmemberaction: "invite"without destination admission context. DM context only hides the separate “not in channel” copy (mentionSuggestionMapping.ts:84-87), whileMentionAutocomplete.tsx:480-488still rendersInvite…. The actual contract forbids adding DM members (features/channels/lib/channelMemberAdmission.ts:22-24), and submit instead reaches a no-invite Send anyway dialog (NonMemberMentionDialog.tsx:49-80). The picker therefore promises a state transition the send path cannot perform.- Author action: derive the presented action from destination membership mutability/add-member authority; add a DM picker→dialog/send regression proving invite side effects and signed recipient tags.
- Verification owner: product/UI rendered-flow re-review.
-
The five-second expiry expires a flag, not cached permission.
useMentionEvidence.ts:28-32,47-52marks verification failed after five seconds, butbuildMentionCandidates.ts:246-273computesallowedfrom still-ready cached directory/policy evidence before consultingverificationFailed. An expired allowed agent therefore remains selectable asMention. An executable production-seam counterexample expectedunavailablebut returnedmention; the existing test atMentionAutocomplete.test.mjs:611-648asserts only the flag, not candidate admission.- Author action: make expiry invalidate presentation and selection even with cached allowed data, or bind fresh evidence to the request; test
mention → unavailable/checkingat expiry and blocked selection until retry succeeds. - Verification owner: systems/policy delta re-review and expiry-gate mutation check.
- Author action: make expiry invalidate presentation and selection even with cached allowed data, or bind fresh evidence to the request; test
-
Unavailable reasons are hover-only and not reliably accessible. Policy denial and verification failure both visibly collapse to
Unavailable(MentionAutocomplete.tsx:480-488); the distinguishing reason exists only intitleon a disabled row (:387-391). Touch and keyboard/screen-reader users cannot reliably learn whether Retry can help, contrary to the PR’s “reason shown” behavior and the repository’s WCAG AA constraint.- Author action: persistently render or accessibly disclose the reason and associate it with the row/retry without duplicate tab stops; test denied and verification-failure keyboard/AX states.
- Verification owner: product/UI keyboard and accessibility re-review.
Evidence/gates: full Desktop unit package 6059/6059 passed; pnpm check, pnpm typecheck, file-size gate, and git diff --check passed; current applicable GitHub Desktop/E2E/build/security/DCO checks were terminal green. Local checkout remained clean at the exact head. Historical mock-bridge screenshot corroborates the missing visible reason but is not exact-head native authorization evidence. Real native keyboard/AX and exact-head native workflow receipts remain a confidence gap; that gap is non-blocking by itself and does not create author rework beyond the source-backed defects above.
Green gates are evidence. They do not persuade the dungeon to stop containing spikes.
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
|
🤖 Addressed review 5159912572 at exact new head Closes all three findings in review 5159912572: destination-aware DM action (no impossible Invite promise), expired/pending/failed evidence blocks cached selection, and persistent accessible reasons with one native Retry control. Installed relay-only nonmember rows now retain exact-key reason/Checking/Retry overlays without adding discovery candidates or moving identities, labels, order or index. Retry is a fresh evidence lookup, not a replacement chooser request (including installed empty/error results); text change/reopen discovers a new list. Independent latest P2/P3 closure is retained. Reconciled local evidence (not rerun for publication): 6,153/6,153 Desktop package and 102/102 focused on composed production; TypeScript and actual desktop-check passed; exact per-PR base/tree size gates passed. Broad Chromium/mock-bridge run remains 233/234 before the test-only formatting correction, followed by the full affected 66/66. Final test-only B1 delta passed full remote-owned-mentions 20/20 and real lifecycle/authority 16/16. Production/dependencies/build/every served file are equivalent across those test-only deltas. No fresh 234/234, union total, repository-wide just ci, live-relay, native Tauri/WebKit/IME/VoiceOver certification, or current-head CI success is claimed. Historical c98 draft failure remains unattributed, not proof of autofocus causation and not a permanent gate after the accepted present-invariant proof. Existing standalone-stack limitations remain. Source evidence: OUTBOX/MENTION_10055/REPORT.md and OUTBOX/MENTION_C0F1/REPORT.md (the latter supersedes only the former B1 blocker disposition). Please re-review the latest head above, not the superseded published head. This response records author closure and requests verification; it is not reviewer approval. |
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — APPROVE at replacement head 479bbe0f17d7635c48e00b5f7280350b1d8b2dba (base 0464384b676b7457f51876484e9beac998b690d5). The prior review at f1af1b88… is superseded.
The full A Team re-review confirms all three prior blockers are closed:
-
DM action truthfulness:
useMentions.ts:81,295-321now supplies destination add-member authority, andbuildMentionCandidates.ts:280-295emitsinviteonly when that authority is true; otherwise it emits actionablemention-without-invite.useMentionSendFlow.authority.test.mjs:253+proves a DM sends directly with zero add-member effects and the exact mention recipient. Browser coverage also checks no dialog/add-member side effect and signedh/ptags. -
Expired cached permission: pending evidence becomes
checkingand failed/expired evidence becomesunavailablebefore cachedalloweddata can admit the row (buildMentionCandidates.ts:280-295). Exact live-row settlement is fenced inuseMentions.ts:491-575. A causal mutation removing the expiry branch made both member and relay-only nonmember journey tests fail behaviorally (mention/inviteinstead ofunavailable), then restoration returned a clean tree. -
Accessible unavailable reasons: reason text is persistently rendered and referenced through
aria-describedbyby both the disabled choice and Retry control (MentionAutocomplete.tsx:389-405,522-554). Retry transfers focus to the composer before unmounting. Unit accessibility assertions cover policy denial and lookup failure.
Exact-head validation: full Desktop unit package 6069/6069 passed; four affected composed suites 86/86 passed; pnpm check, pnpm typecheck, E2E build/type compilation, file-size gate, and git diff --check passed. Windows/macOS builds, Desktop Core, relay/integration suites, security scanners, and DCO are green. Local exact-head worktrees were clean, and the live PR remained at the pinned SHA immediately before this review.
Confidence gap / ownership: Desktop Smoke E2E (3) is red from an exhausted assertion in unchanged messaging.spec.ts:2437; two other tests passed on retry and were classified flaky. The reviewed source and run do not establish PR causation. An optional focused rerun was blocked by reviewer port contention. Author action: none. Verification owner: CI/reviewer for rerun or causal triage and continued observation of picker timing.
The code now says what it means. A rare and beautiful dungeon event.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: 0464384b676b7457f51876484e9beac998b690d5..479bbe0f17d7635c48e00b5f7280350b1d8b2dba (exact live head 479bbe0f17d7635c48e00b5f7280350b1d8b2dba)
Risk: high — desktop mention eligibility crosses live authorization evidence, destination member-admission authority, selection, signed recipients, and accessibility semantics.
Both assigned lanes completed and agree that all three prior blockers are closed.
- DM action truthfulness:
useMentions.ts:81,295-321supplies destination admission fromuseCanAddChannelMembers;buildMentionCandidates.ts:280-295emitsinviteonly when that authority is true, otherwise actionablemention-without-invite. DM tests prove direct send, zero add-member effects, and exact signed recipient tags (useMentionSendFlow.authority.test.mjs:253+;mentions.spec.ts:3255+). - Expired cached permission: pending/failed evidence becomes
checking/unavailablebefore cachedallowedcan admit it (buildMentionCandidates.ts:280-295), with exact live overlays and current-key/action admission inuseMentions.ts:491-575. Removing the productionverificationFailedbranch caused both member and relay-only nonmember journeys to fail behaviorally; restoration returned green. - Accessible unavailable reasons: persistent reason text describes both the disabled choice and single native Retry control, and Retry restores composer focus (
MentionAutocomplete.tsx:389-405,522-554). Unit AX assertions cover policy denial and lookup failure.
Author action: none.
Verification owner: CI/reviewer owns rerun or causal triage of the unrelated Desktop smoke failure; optional isolated Playwright/native keyboard observation remains reviewer/release work.
Exact-head validation on clean trees:
- Full Desktop unit package passed 6,069/6,069; four affected production suites passed 86/86.
pnpm check,pnpm typecheck, E2E build, file-size gate, andgit diff --checkpassed.- Expiry mutation failed the intended two production journeys and was restored.
- Exact-head Desktop Core, smoke shards 1/2/4, relay E2E, both integration shards, macOS/Windows builds, DCO, and security checks are green.
CI confidence gap, not author rework: Desktop smoke shard 3 exhausted one assertion in unchanged messaging.spec.ts:2437 and classified two other rows flaky after retry. The PR does not change that exhausted file/spec, so no PR-caused required-gate failure is established. A PR-relevant keyboard/Options row missed once and passed its immediate retry in 2.5 seconds; no reproducible product defect was established.
Manual/native evidence: no native keyboard/AX run and a focused local Playwright attempt was blocked by occupied port 4173. Source, AX tests, full package checks, and exact-head CI are proportionate; these are reviewer/tooling gaps.
Residual risk: transient picker timing remains imperfectly characterized. No unresolved authorization, recipient-integrity, DM truthfulness, or accessibility defect was found. Any new head invalidates this approval.

🤖
Summary
The @ mention list now tells you, live, whether each person can actually be mentioned — and lets you retry when a check fails.
Part of the mention-chooser stack on the shared #7190 recovery prerequisite: #7190 → #7196 → #7323 → #7197 → #7239 → #7240. This feature stack remains separate from #7191 → #7192. This PR builds directly on #7197.
Related issue
Continues the merged mention-editor work from #7124 (authorize remote mentions at publication) and #7128 (preserve spacing after multi-word mentions). No separate tracking issue for this slice.
Testing
Current accepted replay (2026-09-09)
Published head
479bbe0f17d7635c48e00b5f7280350b1d8b2dba, treeb5a4699121465cdc8116570931182373d65f834b, base0464384b676b7457f51876484e9beac998b690d5. Only #7239, #7240 and #7542 were advanced in one atomic exact-lease push; the other six stack heads are unchanged.Closes all three findings in review 5159912572: destination-aware DM action (no impossible Invite promise), expired/pending/failed evidence blocks cached selection, and persistent accessible reasons with one native Retry control. Installed relay-only nonmember rows now retain exact-key reason/Checking/Retry overlays without adding discovery candidates or moving identities, labels, order or index. Retry is a fresh evidence lookup, not a replacement chooser request (including installed empty/error results); text change/reopen discovers a new list. Independent latest P2/P3 closure is retained.
Reconciled local evidence (not rerun for publication): 6,153/6,153 Desktop package and 102/102 focused on composed production; TypeScript and actual desktop-check passed; exact per-PR base/tree size gates passed. Broad Chromium/mock-bridge run remains 233/234 before the test-only formatting correction, followed by the full affected 66/66. Final test-only B1 delta passed full remote-owned-mentions 20/20 and real lifecycle/authority 16/16. Production/dependencies/build/every served file are equivalent across those test-only deltas. No fresh 234/234, union total, repository-wide just ci, live-relay, native Tauri/WebKit/IME/VoiceOver certification, or current-head CI success is claimed. Historical c98 draft failure remains unattributed, not proof of autofocus causation and not a permanent gate after the accepted present-invariant proof. Existing standalone-stack limitations remain. Source evidence: OUTBOX/MENTION_10055/REPORT.md and OUTBOX/MENTION_C0F1/REPORT.md (the latter supersedes only the former B1 blocker disposition).
Current-head CI and fresh review are pending verification, not covered by old approvals. The evidence and head-specific CI statements below are preserved historical records, superseded by this block for the current head.
mentionPresentation.test.mjsandMentionAutocomplete.test.mjscover the status labels, in-place transitions, request-bound retry, and the 5-second verification expiry;mentionAdmissionJourney.test.mjsadds the presentation/admission journey.mention-picker.spec.tsandmentions.spec.tscover live status, reason, and presence on stable rows, explicit Retry issuing a fresh request, verification expiry, and no readiness while a people search is unfinished.docs/mention-editor.mddocuments the availability-label behavior.Historical capture state (not these heads)
The following original captions/links are retained as historical capture evidence only; none is relabeled as this restack.
Preserved action-label comparison
Historical mock-bridge captures from PR5
0426fac7and PR622b1d536. These still illustrate the action labels (the new stable-list lifecycle is different); they are not screenshots of the repaired publication heads. Different background messages and tray heights make this a behavior comparison, not an identical-background pixel comparison. Images do not prove authorization or delivery.Before — available agent only
At PR5

0426fac7, only the available agent appears.After — available actions and unresolved members
At PR6

22b1d536, channel members with denied or unresolved access remain visible as Unavailable or Checking access…, beside Invite….Integrated stable-picker visuals
Captured integrated f638b9cb — not standalone PR5 and not a capture of this correction. These still illustrate stable rows, action labels and next-open ranking; the earlier correction changed null-destination readiness and fixtures, not those pictured channel states. The duplicate-row image is before Tab, as captioned. No image proves access or delivery.
Extraction validation update
3825f894d08acb5a12d609fbb100da7d982274f2. Core, Smoke 1–3 (including the repaired hover test), builds, relay and both integration shards passed; DCO passed and GitHub reports no merge conflict. Smoke 4 failedworkflow-local-controls.spec.ts:267(initial inspector-click timeout; built-in retries fail the geometry assertion at :297). The subsequently authorized fixture repair and new gates are recorded below.3825f894d08acb5a12d609fbb100da7d982274f2, same Darwin Playwright configuration and same production served artifact, also fails by 438 pixels. Actual, expected and diff PNGs are each byte-identical to candidate fullspec artifacts. This screenshot test does not call the modified helper and its setup/capture is unchanged. Local HTTP content was verified against all 459 JS/CSS/index files; repair tree is exactly the tested2a8f9068e9748f7a908b77097c98097b6c91fc7f. This is an inherited Darwin baseline limitation, separate from target repair proof; official Linux CI remains the intended gate. No baseline update, threshold relaxation or icon hiding.27593788c4f21a74b1ccb994b2e926bf69b5cfcd. Every other job passed — Desktop Core, Smoke 4 with the repaired workflow-local-controls suite fully green (including the previously failing structured-conditions test), Smokes 2–4, both integration shards, builds, DCO — except Smoke 1: the unrelatedchannels.spec.ts:1005test "drops an expanded DM after the first message fails" timed out at line 1066 because a persistent "Message failed to send" error toast intercepted the send button for the full 30s on all three attempts (331 passed / 2 flaky / 2 skipped in the shard). That spec is untouched by this test-only change, and the identical test passed in 3.3s on fix(desktop): order same-name agents using your recent choices #7240's superset run 34276688671 eight minutes earlier — an environment flake, not a regression from this PR. DCO passes; GitHub reports MERGEABLE. No whole-stack ready/merge claim.pnpm build:e2eproduction build; the repaired test passes in 6.2s beside its startup-failure sibling. Desktop lint passes with only pre-existing warnings in untouched files.be1d594f466434cdaeabd6aa4999b25b0a6a027b. The repaired toast test passed — Smoke 1, Desktop Core, Smokes 3–4, both integration shards, E2E relay, builds and DCO all green — except Smoke 2: the unrelatedmentions.spec.ts:1885"relay-agent directory errors fail closed and recover after a fresh fetch (explicit picker)" exhausted its three attempts at theexpect(input).toBeEmpty()assertion (line 1939) and the dependent aggregate Desktop job followed (358 passed / 1 flaky in the shard). That spec is untouched by this test-only change: the same assertion failed its first attempt and recovered on the built-in retry on the prior head's 34276689304 Smoke 2, and needed its final retry on fix(desktop): order same-name agents using your recent choices #7240's prior 34276688671 Smoke 2 — an inherited retry-luck flake, not a regression from this PR; the identical superset tree passed Smoke 2 fully on 34363497199. DCO passes; GitHub reports MERGEABLE. No whole-stack ready/merge claim.mentions.spec.ts:1885explicit-picker setup clear now sends nativeControlOrMeta+AandBackspacewith an immediatetoBeEmpty()gate instead offill(""), whose programmatic selectAll could lose the selection to ProseMirror's selection sync in CI and leave the seeded "@quinn" behind until the picker-empty assertion exhausted its retries. Every original directory-error, disabled-row, Tab-completion and retry-recovery assertion is retained; no production code, timeout or mock change. Self-reviewed eight-line test-only delta (+8/−1).f1af1b88c6458d40080529e8d71a0bdec1868a78: run 34371490090 (CI, 15:37:48Z). The prior terminal FAILURE 34363489121 atbe1d594f466434cdaeabd6aa4999b25b0a6a027bremains the last completed run. No whole-stack ready/merge claim.