Skip to content

Refine direct message opening - #107

Merged
wesbillman merged 3 commits into
mainfrom
clay/messages-sidebar-actions
Sep 25, 2026
Merged

wesbillman merged 3 commits into
mainfrom
clay/messages-sidebar-actions

Conversation

@delkc

@delkc delkc commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Why

New Message can create and send to a direct message, but it cannot open the selected conversation before someone writes a message. Recipient lookup also misses exact public keys, and Enter selects the first result before keyboard navigation.

What

  • Add an explicit Open conversation action for one-to-one and group direct messages without sending a message.
  • Match full public keys, rank search results by relationship, and require arrow navigation before Enter selects a recipient.
  • Keep progressive results tied to exact identities as more directory pages arrive.

How

This reuses the direct-message open capability, verified participant discovery, durable send recovery, and hide lifecycle already on main. It does not add another protocol, sidebar, persistence, or lifecycle owner.

Opening clears only the saved recipient selection. Unsent composer text remains available if someone returns to New Message.

Exact-key lookup uses an author-scoped profile read. Short key fragments do not match. Search ranking prefers an existing one-to-one conversation, prior direct-message participants, managed agents, shared-channel members, then other eligible people. New pages append without retargeting keyboard selection.

Risk

The production change is limited to New Message selection and navigation. It calls the same verified open path used by first-message send and profile Message actions. It does not change DM hide commands or sidebar removal policy.

Testing

  • bin/pnpm test:browser:ci tests/browser/new-message.spec.mjs --project chromium --project webkit --no-deps — 4 passed.
  • The guarded pre-push gate passed 96 files and 1,415 tests with VITEST_MAX_WORKERS=1, plus all design-system guards.
  • No live relay or packaged desktop test in this revision.

Bigger picture

PRs #156 and #92 landed the original New Message and confirmed hide work. This PR now keeps only the recipient and open-without-send behavior that remained after those merges.

Generated with Goose

@delkc
delkc force-pushed the clay/messages-sidebar-actions branch 2 times, most recently from fe0afc9 to 18b2a5f Compare September 22, 2026 17:00

@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.

Reviewed head 18b2a5f478e434fa298aa4119f477b553c0f28aa against base 319635e1ca0696fc06101f9d5c294fb12bfcbc23. Changes requested.

Two P2 blockers:

  1. Hidden DMs reappear on startup when the visibility snapshot beats channel metadata.
  2. The only Hide action is excluded from keyboard navigation, with no reachable alternative.

See the inline findings. Exit criteria: retain hide state through both discovery response orders without suppressing non-DMs, and make Hide reachable/operable with the keyboard. Preserve canonical exact-recipient reopen and membership semantics.

Validation

  • Reviewed the session/discovery lifecycle, canonical roster admission, purpose-bound signing, transport failure semantics and UI integration with complementary reviewers.
  • At this exact head with production source unchanged and a temporary two-case regression test added, the full bin/pnpm exec vitest run run passed all 2,069 existing tests plus the metadata-first control. The hidden-snapshot-first regression failed: the supposedly hidden DM remained in the ready channel list. These are real session/store/discovery tests with signed fixtures and explicitly gated network completions, not browser or native interaction evidence.
  • Existing CI is not green: Chromium channel-opening measurement was 117.9ms against a <100ms warm-switch bound; WebKit shard 1 failed the saved-dark-document reload test with Importing binding name 'n' is not found. JavaScript, Rust/tool integration, both Chromium journey shards and WebKit shard 2 passed. I have not attributed these CI failures to this diff or established that they are pre-existing/flaky.
  • I did not exercise a live identity or an actual native UI in this review.

Non-blocking diagnostic correction

dev/relay-broker.mjs:1284-1293 calls every SocketRequestError(sent=false) a relay rejection and returns 422 with sent:true, including locally unsent capacity/pre-dispatch-timeout errors. Preserve local non-delivery and distinguish it from an actual correlated negative relay receipt. This is a misleading error/provenance issue, not an established delivery-safety blocker: the current DM transport treats both as proven failure and the UI does not retry automatically.

Comment thread src/features/relay/store.ts Outdated
Comment thread src/bundled/channels/ChannelsPage.tsx Outdated
@delkc
delkc force-pushed the clay/messages-sidebar-actions branch from 18b2a5f to e2553a1 Compare September 22, 2026 18:21
@delkc
delkc marked this pull request as ready for review September 22, 2026 18:39
@delkc
delkc requested review from a team and comp615 as code owners September 22, 2026 18:39
@delkc
delkc requested a review from wesbillman September 22, 2026 18:42

@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.

Reviewed head e2553a1f0724a6f759ab7bf0ce9d67add9296ec8 against base 298a50a9861c19d33e45b2b52bdadb81b128f15d.

Changes requested

P2: Keep the eight-recipient conversation submittable. NewMessageView.tsx:496 disables the recipient input when selected.length >= 8, but that same input's Enter handler is the only user path to open() (lines 428–433, 509). There is no form submission or Open button. Select eight distinct recipients through the advertised group-DM flow: the input becomes disabled, cannot receive Enter, and the screen still says “Press Enter to open this conversation.” Removing someone is the only way to regain submission, so the supported maximum-size group cannot be opened.

Keep submission reachable at the limit while preventing a ninth recipient, without relaxing the host limit. Add a regression that selects eight exact recipients and submits that exact set through a real enabled user control; also verify a ninth cannot be added. A synthetic key event dispatched to a disabled input would not prove this contract.

P2: Preserve the highlighted recipient by identity across asynchronous search expansion. NewMessageView.tsx:160–172 merges a second result set and re-sorts it, but highlighted remains an array index. The existing “dr” fixture demonstrates the ordering: the first 25 results include Dr. 0; the later result Drafty sorts before it under lines 302–328. Arrow to Dr. 0 while the extended read is pending, release that read, then press Enter: lines 430–432 now select Drafty, not the identity the user highlighted. The current selection-preservation test resolves with an empty result and misses this case. Anchor keyboard selection to the pubkey (or deliberately clear it if no longer eligible), and test a nonempty deferred result that inserts ahead of the active identity. No extra user input should silently retarget a DM recipient.

P2: Make Hide's disabled state match its global in-flight guard. ChannelsPage.tsx:755–756 drops every hide request while any DM is being hidden, but the hover button and keyboard menu disable only the matching row (lines 833, 845). Hold Hide A pending, then activate the still-enabled Hide B: the action silently returns without a request, feedback, or eventual hide. The smallest fix is to disable all Hide controls while that global guard is active; concurrent per-channel machinery is not needed. Cover the pending and recovery states with a deferred first hide.

Prior review disposition and scope

The prior startup visibility and keyboard-Hide blockers are resolved in the reviewed code paths: visibility IDs survive either metadata/snapshot order, the store filters only DM visibility while retaining authoritative membership, and the existing focusable row now opens Hide through Shift+F10/ContextMenu. The broker diagnostic fix also distinguishes receipt-less proven-unsent failures from correlated relay rejection. Exact-recipient canonical reopen and purpose-bound signing remain intact.

Review covered the full current feature diff across sidebar/search/menu integration, session lifecycle and discovery/store authority, broker/transport/socket receipt provenance, and the added regression tests. Read-only Blox source review with independent UI and authority lanes; no checkout, build, tests, native interaction, or live writes were run for this review.

Existing exact-head CI evidence

JavaScript, Rust/tool integration, both Chromium journey shards, WebKit shard 2, DCO and security checks passed. CI is not green: WebKit shard 1 failed the saved-dark-document startup case with missing import binding n; browser measurements measured warm channel visibility at 120.4 ms against the <100 ms gate (two later cases did not run). Windows native validation was skipped. Separately, ChannelsPage.tsx:856–863 replaces stable row callbacks with inline closures, defeating the existing ChannelSidebarItem memo boundary on parent updates. Preserve stable callbacks when addressing the sidebar performance gate; source alone does not establish that this caused the measured failure. Those CI failures still require resolution before merge; this review does not establish they are unrelated or flaky, and does not authorize relaxing their assertions.

@delkc
delkc force-pushed the clay/messages-sidebar-actions branch from e2553a1 to 7501d11 Compare September 23, 2026 19:58

@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.

Changes requested: one P2 hide/reopen integration regression, detailed inline. Reviewed 7501d114d1410fe6ace89ee4c20b6ab84571666a against base/merge-base 1f84835bbb7bbb83af8a4b45094ca3414ffa7a27.

Merge criteria: reconnect authoritative DM visibility to startup and reconcile the retained local hide state on reopen, with coverage for server-hidden startup, hide → reload → reopen → reload, existing local hides, and both discovery/snapshot orders. The eight-recipient submission, stable search selection, keyboard Hide and global pending guard are fixed in source.

Validation: read-only Blox source review across page/store/session/persistence and command/receipt boundaries, plus an independent CI-fixture diagnosis. No checkout, code execution, tests, builds or live writes. Exact-head hosted CI is not green: the DM-hide journey fails in Chromium and WebKit; the fixture still uses non-UUID DM IDs and must model the new kind-41012 command path without weakening production validation. Warm switching measured 118.9 ms against the <100 ms gate. Those failures remain merge gates; they are not a live reproduction of the inline production defect. JavaScript, Rust/tool integration, both shard-1 journeys, DCO and security checks passed; Windows native validation was skipped.

Comment thread src/bundled/channels/ChannelsPage.tsx
@delkc
delkc force-pushed the clay/messages-sidebar-actions branch from 7501d11 to 835fbfa Compare September 25, 2026 13:51
@delkc delkc changed the title Add direct message actions Refine direct message opening Sep 25, 2026

@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.

Changes requested: one P2 recipient-search contract defect, detailed inline. Merge criterion: local public-key matching must require a complete key, with a cached-recipient regression covering the 8-character boundary (and a longer incomplete prefix) while retaining full-key lookup.

The previous hide/startup/reopen blocker is resolved by the shared owners now on main; this revision reuses their verified DM-open path.

Validation: source-only review plus independent challenge, with no PR-code execution. Existing CI run 36143629831 passed on a merge tree identical to this head: 3,792 JS tests, Chromium/WebKit journeys, measurements and Rust. That run uses base 01523e2c1781da1cd84620e61188df6c3d365938, not newer main bebcd66e. Windows was skipped; no live-relay or packaged-desktop acceptance. The new Open conversation button has single-recipient component coverage, but no direct browser/group-open coverage; draft retention and relationship buckets also lack direct assertions. Those are disclosed gaps, not additional blockers.

Comment thread src/features/direct-messages/usePeople.ts Outdated
Signed-off-by: Clay Delk <clay.delk@gmail.com>
Signed-off-by: Clay Delk <clay.delk@gmail.com>
Signed-off-by: Clay Delk <clay.delk@gmail.com>
@delkc
delkc force-pushed the clay/messages-sidebar-actions branch from 835fbfa to 8fb04cc Compare September 25, 2026 15:26
@delkc

delkc commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

🤖 The latest P2 is addressed at 8fb04cc2: cached public-key matches now require exact 64-character equality, with regressions for 8- and 40-character incomplete prefixes plus the retained full-key case. The branch is rebased onto current main. Guarded pre-push passed 96 files / 1,417 tests and all design-system checks; hosted CI is running. Please re-review when ready.

@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.

Focused re-review complete: no remaining blocking findings at 8fb04cc2a94abf53b9fd0d97ba790d1ea5774e2f against bebcd66e0adb27efa2c320f2bf72b8f3e63a8a45. The previous P2 is resolved: cached and fetched recipients use full 64-hex equality, and the regression now rejects 8- and 40-character prefixes while retaining complete-key lookup. Independent source challenge confirms the test exercises the old failure. The rebase leaves the previously reviewed feature commits unchanged.

Existing CI run 36154182376, attempt 1, passed: 3,794 JavaScript tests, 347 browser journeys per engine, seven measurements and Rust/tool checks. Tested merge 70ebac7b6e876982c8a59a11cd845c9d76cac28d has the pinned base/head parents and a tree identical to this head. No local tests or CI reruns were performed.

Unchanged, non-blocking follow-up: uncached lookup accepts lowercase hex only; uppercase hex is cache-dependent, and NIP-19 input is unsupported. This is not a regression in the agreed prefix fix. Newer main integration, live-relay and packaged-native acceptance remain unverified; Windows CI was skipped. This COMMENT is not approval and does not dismiss the historical changes-requested review.

@wesbillman
wesbillman merged commit e44b1e4 into main Sep 25, 2026
12 checks passed
@wesbillman
wesbillman deleted the clay/messages-sidebar-actions branch September 25, 2026 17:33
cynfria pushed a commit that referenced this pull request Sep 25, 2026
…sh-pr1

* origin/main:
  Refine direct message opening (#107)
  feat(messages): report messages to community moderators (#255)
  perf(channels): stop rerendering message rows after each channel switch (#269)

Signed-off-by: Peppermint Butler <29b529ff43be424db89f51bff58a38cf86942acbffff98a14ce50b38c2a57d8f@buzz.block.builderlab.xyz>
johnmatthewtennant pushed a commit that referenced this pull request Sep 25, 2026
* origin/main:
  Explain missing Pi provider models (#263)
  Browse Goose models and enter provider API keys (#230)
  test(agents): check model lookup Cancel by visible text (#259)
  Ask before mentioning people outside the channel (#257)
  Refine direct message opening (#107)
  feat(messages): report messages to community moderators (#255)

Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz>
zrmarley added a commit that referenced this pull request Sep 25, 2026
…-image

* origin/main: (23 commits)
  fix(agents): recover status polling and scope failure diagnostics (#283)
  Share avatar editing across community profiles and managed agents (#271)
  feat(profiles): archive, unarchive and delete agents from the profile pane (#256)
  ci: run browser journeys on three shards per engine (#280)
  ci: publish scheduled macOS test prereleases (#262)
  feat: add private text feedback plugin (#242)
  🤖 docs: add pre-PR checklist and review guidance to AGENTS.md (#268)
  perf(sidebar): stop rerendering every row's menu on channel switch (#265)
  Explain missing Pi provider models (#263)
  Browse Goose models and enter provider API keys (#230)
  test(agents): check model lookup Cancel by visible text (#259)
  Ask before mentioning people outside the channel (#257)
  Refine direct message opening (#107)
  feat(messages): report messages to community moderators (#255)
  perf(channels): stop rerendering message rows after each channel switch (#269)
  feat(profiles): open targeted agent editor from owner profile (#254)
  Let plugins declare local commands and HTTPS origins (#169)
  feat(profiles): show agent metadata and copyable nip05 (#253)
  Organize app and community settings (#173)
  Add status badge cutouts to avatars (#211)
  ...
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.

2 participants