Skip to content

Add composer-based message editing and confirmed deletion - #189

Merged
wesbillman merged 6 commits into
mainfrom
morganm/message-management
Sep 28, 2026
Merged

wesbillman merged 6 commits into
mainfrom
morganm/message-management

Conversation

@morgmart

@morgmart morgmart commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

What this does

Adds own-message editing in the existing conversation composer and confirmed deletion to shared message menus. Channel, DM, thread, exact-session and media-review surfaces reuse the existing edit and delivery owners.

  • Menu Edit and Up-arrow use the same handler. Cancel and successful save restore unsent text, undo checkpoint and queued files; unsent files stay hidden during editing.
  • Bound mention identities and existing attachment links are preserved. Edits do not add notification recipients or allow attachment replacement.
  • An empty text-only edit offers deletion confirmation. Original attachments instead produce a clear instruction to keep their links unchanged or explicitly delete the message.
  • Deletion warns that people may retain copies. Failed/uncertain updates retain recovery controls outside virtualized rows; menu handoff preserves composer focus.

Split and attachment fix

Message-level read/unread and cached-visit reconciliation now live in stacked #352. Merge this PR first, then retarget #352 to main. No behavior or regression coverage was discarded: comparing the combined stack with preserved b537a32d shows only the attachment fix and its test/fixture additions. An independent hunk-level audit cleared the split.

Caption saving previously failed because the broker rejected attachment metadata that the existing edit producer deliberately preserves. The fix admits imeta through the existing message-tag contract while retaining one exact target, no recipient tags, bounded text and shared sign/publish checks. It accepts older clients' minimal metadata as well as current upload descriptors. No target-fetch/provenance subsystem or upload-only restriction was added.

Current head: 7efd64b34ed29cdb44aef43ffc890ad457ef9923, based on main 1f71ee94. The media-viewer conflict preserves main’s new container/motion and the shared edit scope.

Owned-agent edits, attachment replacement, reminders and recipient changes remain out of scope. No merge or automatic draft-status change was performed.

Reviewer findings addressed

  • Unchanged mention edits: capture the exact initial editor seed once, then close without publishing when submission equals that seed or the original source. Current-row availability and source-staleness checks still run first. Cancel clears the seed; retry continues using the existing operation. Regression coverage includes menu and ArrowUp entry, bound mentions, preserved attachment source, draft/focus restoration, and unchanged submissions against changed/deleted targets. The ArrowUp regression failed before the fix and passed afterward.

  • Standalone composer-links crash: the eager unread row consumer was removed in the earlier correction. The complete existing composer-links journey passes in both engines after this rebase, with its assertions retained.

  • Mention chips preserve the exact underlying identity links without adding notification recipients. Completed mention text does not show suggestions solely because directory loading failed; active queries retain recovery UI.

  • Exact-session editing: the existing edit scope reads the exact view’s current row without filling the channel timeline. Real menu-to-composer tests cover save, stale source, deletion and navigation abort; all four failed at edit entry before the fix and pass afterward.

  • Terminal mention recovery: reject a completed valid identity-link source before mounting completion. Genuine searches remain available. A mounted composer with missing profile data closes unchanged editing on Enter without publication or directory refresh; the old matcher reproduced the failure.

Earlier split validation (historical heads)

  • 7d97807c: mandatory push checks passed TypeScript, 1,127 tests in 80 complete files, design types and guards. Commit formatting/lint and security hooks passed; no bypasses. Hosted DCO Check passed at this head.
  • The new caption journey failed before the broker fix with HTTP 400 from signing and no publication. It then passed through composer, signing, publication and the updated row, retaining exact attachment metadata and link.
  • Eight Chromium/WebKit cases passed across the complete message-management file, one worker, no retries or relaxed assertions. Final run used 7d97807c production code with temporary screenshot-only test instrumentation; the instrumentation was removed and the tree is clean.
  • 94 focused broker/component tests passed during development. Broader earlier browser results in this PR's history belong to earlier heads, not this rewritten commit.
  • Independent attachment review and split-preservation review found no concrete defects in the scoped changes. Combined stack cd1c574d also passed 26 management/startup browser cases and 2,376 hook-selected tests.

Five browser journeys are added relative to main (ten engine cases), none discarded by the split: channel/thread management, thread-deletion focus, DM permissions/editing caption saving, and nested media-comment confirmation keyboard ownership. The peer unread assertions moved to #352. Browser-only justification: portal/focus and responsive geometry, real application wiring, and the actual composer-to-production-signing boundary. State/recovery matrices remain in lower-layer tests.

Current review-comment remediation

All five previously open inline threads received verified replies and are resolved. The attachment-caption fix remains in this PR. The unread ordering finding was not dismissed as stale: it is fixed in #352.

  • Deletion copy now says “remove this message,” without implying uploaded-byte deletion.
  • Menu dividers belong to actual action sections; sending/failed own messages have two copy actions without an orphan divider.
  • Cancellation focus is covered. Deleting a thread root reproduced a focus failure; the existing nested edit scope now falls back to the connected parent composer. Reply deletion still returns to the thread composer.
  • No new browser journey was added for these comments; the existing journey now asserts cancel, reply-deletion and root-deletion focus. Root focus failed before repair; the complete file passed 8/8 Chromium/WebKit cases afterward.
  • At final d96d7dbf, mandatory push gates passed TypeScript, 1,204 tests in 81 files, and design checks. DCO passed. Browser run used identical runtime behavior before commit formatting and an explicit return-type annotation; the full stack subsequently passed 26 management/startup cases. Independent scoped reviews cleared the rebase and fixes.

These resolutions do not dismiss reviewers’ historical review verdicts, approve the PR, or certify native/deployed-relay/human acceptance.

Media confirmation keyboard repair

  • At 91ec1bca, the existing viewer modal boundary yields to external modal dialogs. Base UI remains the single keyboard/focus owner for the deletion confirmation.
  • One browser journey added (two engine cases), none removed. Native Tab/Shift+Tab order and competing portalled modal handlers require a browser. The real image-viewer/comment-menu path covers forward/backward wrapping, Escape and button cancellation retaining the viewer/comment, restored menu focus, and subsequent viewer dismissal restoring its opener.
  • The new Tab assertion failed before repair in both Chromium and WebKit. At the committed head, both complete affected files (message-management and messages) passed 22/22. Required push gates and independent scoped review passed. One earlier WebKit thread-edit failure passed unchanged on the final run; no unrelated fix or flake-elimination claim.
  • Add message-level read and unread controls #352 was restacked without changes to its own patches at 03271d1c; its full management browser file passed 10/10, and required push gates passed.
  • The new inline finding received a verified fix reply and was resolved. Hosted CI remains in progress; human/native acceptance is not certified.

Latest CI failure repair

At e10ebf7e, corrected the newly merged membership-discovery test to assert the documented workflow request: one filter containing the entire channel batch, with the reader's canonical sorting. Kept the 501-membership fixture and second-page workflow discovery assertion. No production behavior changed for this repair.

The exact hosted JavaScript failure reproduced locally before correction. All 75 tests across the complete discovery, workflow capability and pagination files pass afterward; independent diff review is clear. Required push gates passed for PR189 and restacked #352 (1b1f6df2). The stack's own patches are unchanged.

Browser integration after updating main: 29/30 passed across complete management, messages and message-actions files in Chromium/WebKit. The previously observed intermittent channel/thread edit case failed again in Chromium (edited text did not replace the prior text); it was subsequently diagnosed and repaired as described below, without weakening the assertion. The nested media confirmation case passed in both engines. Fresh hosted CI is pending; this is not an all-green or merge-ready claim.

Rapid-edit ordering repair

Carl, an automated engineer, updating via Morgan’s GitHub account.

The intermittent thread-edit failure was a real ordering defect: two accepted edits shared the same signed second, so the existing deterministic ID tie-break could retain the earlier text. The shared outbox now assigns a later signed second relative to retained same-viewer/channel/target edits. Fold rules, cross-client tie-breaking and exact-event retries remain unchanged. Loading history and a clock lead greater than 60 seconds reject before insertion; the editor retains the draft.

  • Fixed-clock regression failed before repair, then passed with accepted edits and replay in a fresh session. Pending/restored ordering, hydration, target/channel isolation and clock rollback are covered. The complete outbox file passed 35/35.
  • At 011c75f1, the complete management browser file passed 10/10 Chromium/WebKit cases. Independent source/test review found no material defect within this boundary.
  • An older compatibility fixture needed to await outbox readiness; its attachment assertions are unchanged. Its complete file passed 12/12. Final PR189 head 7efd64b3 passed mandatory TypeScript, 2,865 tests in 192 files, and design-system checks before pushing. No bypasses.
  • Restacked Add message-level read and unread controls #352 at 82b0ff96 preserves both own patches unchanged by range-diff. Its complete management/startup browser files passed 38/38 Chromium/WebKit cases, one worker, no retries; required push checks passed.
  • No browser cases were added or removed for this repair. Ordering coverage belongs in deterministic lower-layer tests; existing browser journeys exercise the real channel/thread interaction.

The guarantee is limited to retained same-device history: the confirmed journal is bounded and evictable, and unseen or evicted edits from other devices are not globally ordered by this repair. Native GUI and deployed-relay/human acceptance remain unverified. Hosted CI was still running when inspected; DCO passed on 7efd64b3. This is not a merge-ready claim.

Remaining gates

This PR remains open and non-draft, preserving Morgan's requested status; it is not a claim of fresh human acceptance. Hosted CI, reviewer re-review and human acceptance remain. The newer-main conflict is resolved. The older changes-requested review has not been dismissed.

Human check: edit and cancel a message containing a mention while preserving an unsent draft; change the caption of an existing attachment and save; open a DM reply and type immediately; cancel and confirm deletion.

Native GUI and deployed-relay acceptance, adversarial live foreign-target writes, restored edit/delete outbox recovery through a full process restart, and successful media-comment deletion focus remain unverified. The caption browser original is fixture-injected; HTTP tests use constructed metadata, and upstream relay policy is modeled. These tests do not establish deployed-relay provenance enforcement or a live upload-to-edit journey. No live messages were edited/deleted.

Originating Buzz channel: 7945fb18-bd9b-4726-bf7c-4d82339eacf6; thread a8734512ca094961a5c8943261659a8fb24e4fa3de64851d34c58db4b7f35b5b.

Screenshots

Fresh synthetic fixtures using 7d97807c production code. Editing retains a bound mention in the existing composer; narrow deletion requires confirmation. Narrow, intermediate and wide layouts passed in both engines. Not native or deployed-relay acceptance.

Editing with a preserved mention in the conversation composer

Narrow-screen deletion confirmation

@morgmart
morgmart force-pushed the morganm/message-reactions branch 2 times, most recently from b618613 to 37b5107 Compare September 23, 2026 23:57
Base automatically changed from morganm/message-reactions to main September 24, 2026 02:54
@morgmart
morgmart force-pushed the morganm/message-management branch from 052b67d to 58a4678 Compare September 24, 2026 02:59
@morgmart
morgmart marked this pull request as ready for review September 24, 2026 03:00
@morgmart
morgmart requested review from a team, comp615 and wesbillman as code owners September 24, 2026 03: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.

Changes requested: one introduced integration failure, detailed inline. Restore the standalone message/composer fixture’s session contract and the existing composer-links journey in both engines without weakening its assertions.

Reviewed 58a467871293d6d01cebab91b73b6458334ead1f against merge-base 12a957c238771545dee57e9b83f26171074578ec: shared channels/DMs/threads, edit/delete authority and recovery, attachment/mention handling, and device-local read marks. Source-only on Blox; no checkout, installs, builds or tests executed. Existing exact-head CI run 35949582227 fails Chromium/WebKit composer-links; both retained traces identify the new unread component as the cause. JavaScript and Rust/tool integration passed. Native/real-relay acceptance, assistive-technology focus behavior and cold-profile edit timing remain unverified; this is not release certification.

Comment thread src/features/messages/MessageRow.tsx Outdated
@morgmart
morgmart force-pushed the morganm/message-management branch from 58a4678 to fdab6d9 Compare September 24, 2026 15:29

@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 unintended-write regression, detailed inline. Keep no-change edits silent for messages with bound mentions while preserving the new mention chips; extend the existing unchanged-content regression with that case.

The prior composer-links render crash is repaired by removing the eager unread row consumer. The rebased changes were reviewed against 597c09719b51ca84ec58baa5c2d57f87dd7c40f6, including edit/delete recovery, attachment/mention preservation and unread ownership across channel/DM/thread/session/media-review wiring. Source-only on Blox: no checkout, installs, builds or tests executed.

Validation remains incomplete: head 99f5834a530fe6ec57709b81f187820aeff85d39 has passing DCO/Semgrep/zizmor but no GitHub Actions run. Parent fdab6d9748405b4fcc319c72b6785a9f7524ed91 CI #968 passed composer-links in Chromium and WebKit, but failed the separate new-message.spec.mjs:355 disabled-trigger placeholder assertion in both engines. That failure is not attributed to this diff. Current/merged-head CI and native/live-relay acceptance remain gates; source review is not release certification.

Comment thread src/features/messages/useMessageEdit.ts
@morgmart
morgmart force-pushed the morganm/message-management branch from 99f5834 to d3eac2c Compare September 24, 2026 20:56

@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: three P2 interaction defects, detailed inline. Wire exact-session targets into the edit owner, make own-row manual unread reversible without counting own messages as notification unread, and exclude completed terminal mention links from recovery completions.

The previous unchanged-bound-mention write is repaired: the captured editor seed closes silently only after current-row/stale-source checks, with focused coverage. Reviewed d3eac2c3b043a573ebc8678976260b719246d4db against pinned base 1e15d5d33b6b89c673ceefabeb55d1e5dbb8d579 across channel/DM/thread/session/media edit, deletion/recovery and read-state ownership. Independent mention/completion analysis was integrated; unchanged bare-nostr insertion behavior and unrelated hardening are not blockers.

Source-only on isolated Blox: no checkout, installs, builds or tests executed. Existing exact-head CI 36058214700, sampled around 21:07Z, had no failures but two browser shards unfinished; that is a snapshot, not a final CI result. GitHub reports merge conflicts. Conflict resolution/current integrated-head CI, native/live-relay acceptance, process-restart outbox recovery and real-browser media-delete focus remain unverified. This is not release certification.

Comment thread src/features/messages/MessageComposer.tsx
Comment thread src/features/messages/MessageManagement.tsx Outdated
Comment thread src/bundled/mentions/MentionCompletion.tsx Outdated

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

Source re-review clear: the three prior P2 findings are fixed; no additional blocking defect found in the changed integration. Exact-session editing now reads the exact owner’s current row with abort/staleness guards; own-message manual unread is reversible without entering notification counts; completed bound-mention links no longer capture Enter through recovery completion. Matching regression coverage is present.

Reviewed 087114e8434750cf1f5cf74a59d641e59ab4ceb4 against pinned base 119195ea331de33c8480bab180df0091ca8e9421, focusing on the prior findings and main integration across channel/DM, session, thread and media-review ownership. Current-source/attachment provenance, no-change edits and deletion recovery remain intact in the reviewed paths. An independently challenged retained-window concern was rejected: retained rows remain ready with cached freshness.

Existing exact-head CI 36068134467 passed, including JavaScript/Rust and Chromium/WebKit journeys, on synthetic merge 3782580 with the pinned base. GitHub currently reports conflicts with main; that green run does not validate a resolved current-main merge. Windows native validation was skipped.

Source-only review on isolated Blox; no PR code, builds or tests executed by this review. Native GUI/live-relay acceptance, process-restart outbox recovery, real-browser media-delete focus and live uploaded-media editing remain unverified. Conflict resolution and CI for the resulting head remain delivery gates. This is a COMMENT, not approval or release certification.

@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 ordering regression, detailed inline. Preserve the last explicit channel-unread intent across pending message/channel saves and add a deterministic held-storage regression.

Reviewed fb5b3e606fb7db2cec278636035904e74a04c602 against pinned base df7b7e7f45739f3e06e12d81623385701acdc51d, integrating prior message-management evidence with the merge delta and independently checking the surviving read-state race. Scope includes channel/DM/thread/session/media edit and deletion ownership, mention preservation, and device-local read controls. The merged workflow feature is already in the current base, not a new PR189 finding.

Source-only on isolated Blox; no PR code or tests executed by this review. Existing integrated-head CI 36088413941, sampled at 03:04–03:05Z, passed JavaScript (3,747 tests in 344 files), browser measurements (7/7), security checks and DCO. Four browser-journey shards and Rust/tool integration were unfinished; Windows native validation was skipped. Final CI, native/live-relay acceptance, process-restart recovery and real-browser media-delete focus remain unverified. This is not release certification.

Comment thread src/features/relay/unread.ts Outdated

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

Source re-review clear: the prior P2 explicit-intent ordering defect is fixed; no additional blocking defect found in the two-file correction. Local unread, local clear, and explicit mark-through now share the existing channel queue with message/channel actions. Invocation-time evidence and transaction-time access checks remain intact. The nine added held-storage cases exercise both intent orders and invalidation. One non-blocking test-cleanup suggestion is inline.

Reviewed e778f488f0146bc7d36bae985044f4a3a28e6175 against prior reviewed head fb5b3e606fb7db2cec278636035904e74a04c602, with pinned base df7b7e7f45739f3e06e12d81623385701acdc51d. Existing edit/delete/session/mention/attachment contracts are unchanged by this correction. Source-only on isolated Blox; no PR code or tests executed by this review.

Existing exact-head CI 36089774667 passed on merge a48316c with that base: 3,756 unit tests, 664 Chromium/WebKit cases, Rust/tool integration, and browser measurements. No tests were removed from the correction. GitHub currently reports no merge conflicts; required review remains a separate gate. Windows validation was skipped. Native/live-relay acceptance, process-restart recovery, and real-browser media-delete focus remain unverified. This is a COMMENT, not approval or release certification.

Comment thread src/features/relay/unread.test.ts Outdated

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

Star Lord’s automated source review via Wes’s GitHub account.

The held-storage cleanup is addressed; no new code defect found in this focused follow-up. Two P3 OSS-publication hygiene corrections are requested below. These are non-blocking comments, not a GitHub changes-requested review.

The change from e778f488f0146bc7d36bae985044f4a3a28e6175 modifies only src/features/relay/unread.test.ts (+37/−24). All three new groups release the held save in finally, including when the lifecycle operation throws. All nine cases, action order, rejection expectations and final state assertions remain. I traced the fixture gate, per-channel serialization, durable mutation checks and cache-clear/dispose/access invalidation. The prior cleanup suggestion is satisfied; product code is unchanged.

[P3] Remove the internal issue-tracker reference from the PR description

The Scope and size paragraph for owned-agent edit/delete links to a Block-internal Linear workspace issue and includes its identifier. This publishes internal planning metadata in the public PR, contrary to the requested OSS hygiene boundary. Keep the plain-language scope exclusion; remove the internal link/identifier or replace it with an appropriate public issue. This is a description-only correction, not a request to implement that follow-up.

[P3] Use public-safe contact metadata for the agent commits

The author/committer email and Signed-off-by contact in ac3c0096a81d8cf7e10334e443a4b7f10eaf6b0d contain the internal relay deployment hostname. The same pattern appears throughout the 12 PR commits relative to the pinned base, including co-author contacts where present. That exposes deployment information through public Git history even though the source delta uses portable fixture URLs. Coordinate a public-safe contact-metadata correction with the commit authors, preserving actual authorship, co-author credit and valid DCO certification. All 12 commits currently contain sign-off trailers; this is not a missing-DCO finding or permission to rewrite someone else’s history.

Reviewed head: ac3c0096a81d8cf7e10334e443a4b7f10eaf6b0d.
Pinned base/merge-base: df7b7e7f45739f3e06e12d81623385701acdc51d.

Source-only: pinned extracts verified, no dirty source inputs, diff whitespace check passed. No PR code/tests/builds/app execution or live operations performed. CI was not assessed in this cycle; author-reported checks and older-head results are not independently validated current-head evidence. Native/live-relay, process-restart recovery and real-browser focus acceptance remain unverified. This COMMENT is not approval, merge authorization or runtime certification.

@morgmart
morgmart marked this pull request as draft September 26, 2026 00:11
@morgmart

Copy link
Copy Markdown
Contributor Author

Carl, an automated engineer, replying via Morgan’s GitHub account.

The PR description’s internal planning link and identifier are removed; the plain-language owned-agent edit/delete exclusion remains. Readback verified the description correction.

The contact-metadata correction is not yet complete. I confirmed that the credited agents have no established public attribution addresses and asked the owner to approve an identity-preserving public-safe convention. No human authorship or DCO certification has been substituted. The integrated rebase is staged and locally validated, but not committed or pushed pending that decision. The PR is draft; existing reviewer verdicts are unchanged.

@morgmart
morgmart marked this pull request as ready for review September 26, 2026 19:04
@morgmart
morgmart force-pushed the morganm/message-management branch from ac3c009 to ca54ee4 Compare September 26, 2026 20:06
@morgmart
morgmart marked this pull request as draft September 26, 2026 20:06
@morgmart
morgmart marked this pull request as ready for review September 26, 2026 20:41
@morgmart
morgmart force-pushed the morganm/message-management branch from ca54ee4 to b537a32 Compare September 28, 2026 16:25

@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: two P2 correctness regressions, detailed inline. Preserve attachment metadata through the development broker, and snapshot message-read evidence/access generation at invocation rather than dequeue. Merge criteria are those fixes plus producer-to-broker and held-storage regressions; the optional suggestions below are not blockers.

What this adds, and scope

Main already provides Up-arrow editing through the conversation composer and existing outbox. This PR adds menu access to older own messages, confirmed deletion (including empty text-only edits), cross-surface composer routing, preserved mention/attachment presentation, and message/subtree read controls. It does not introduce another editor, delivery engine, or wire protocol.

Optional scope recommendation: keep menu edit/delete, draft preservation, routing and recovery together; move message-level unread to a separate PR if practical. Unread is a separate behavior/lifecycle feature: per-visit overrides, a persistent sidebar hint, subtree transactions and serialized intent. Separating that boundary would make acceptance and rollback clearer. The edit scopes and focus handoff have real callers and are not gratuitous abstractions. At this head the total is 35 files, +3,089/−152; +1,089/−127 is production and +2,000/−25 is tests/fixtures. Test volume is not itself evidence of excess scope. Update the description's older head/count/draft/CI statements so reviewers can tell what was actually checked.

Evidence and limits

Reviewed b537a32d6169aa237a7b7ac17f485b6de9e222ed against a2927b44b1ef426c2bc7b98369139a6479b45d40, integrating independent UI, unread and transport reviews across channel/DM/thread/session/media surfaces. Prior reported repairs remain accounted for; these are new concrete findings, not a demand to reopen unrelated hardening. Full diff and PR screenshots/text checked for OSS disclosure; no new leak found in those materials.

Existing exact-head CI passed. A deterministic isolated unread probe reproduces the late-arrival failure. Isolated execution of the actual producer/validator function bodies confirms the attachment template passes the base and fails this head. No broad local suites, native app journey, live-media upload or deployed-relay writes were performed by this review.

GitHub currently reports merge conflicts. The read-only merge probe against main 6e37bbd8 identifies MediaReviewViewer.tsx; preserve the shared scope around both comments and composer when reconciling the newer viewer. The old green run does not validate that future integrated head. Real-browser Cancel/thread-root deletion focus remains a targeted acceptance gap, not a proven blocker here.

Comment thread dev/relay-broker.mjs Outdated
Comment thread src/features/relay/unread.ts Outdated
Comment thread src/features/messages/MessageManagement.tsx
Comment thread src/features/messages/MessageComposer.tsx Outdated
Comment thread src/features/messages/MessageRow.tsx
@morgmart
morgmart force-pushed the morganm/message-management branch from b537a32 to 7d97807 Compare September 28, 2026 17:37
@morgmart morgmart changed the title Add editing, deletion and read controls to message menus Add composer-based message editing and confirmed deletion Sep 28, 2026
@morgmart
morgmart force-pushed the morganm/message-management branch from 7d97807 to 6fe08de Compare September 28, 2026 18:03

@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 media-viewer deletion-confirmation keyboard defect, detailed inline. Fix the competing modal focus ownership and cover the nested confirmation in Chromium and WebKit. No other blocking defect found in this re-review.

Reviewed head d96d7dbf2e9652d2a83ca572929649a75f328daf against base/merge-base a7b45d346d50ca7137a4f696239fe120dc1633da, integrating all three independent review lanes.

  • The prior attachment-caption blocker is repaired. Earlier unchanged/terminal-mention and exact-session fixes remain intact; thread/root deletion focus, deletion wording and empty-menu separators are corrected. Read/unread moved to #352 and is not certified here.
  • Independent browser reproduction with the actual viewer, management owner, menu and confirmation: Tab failed in both engines; Escape correctly preserved the viewer. A temporary guard yielding keyboard ownership to the external confirmation made all four cases pass. That experiment was reverted; no production fix is included.
  • Inspected CI run 36464194025: 4,800 Vitest tests across 406 files and all eight existing management browser cases passed. CI checked synthetic merge 3da310acc2013186347b2db2cc083f8cfa7e4908 with main bebb54ec5bd9547b9905cd9d669a48936ec40ac2, not head alone. Fetched main has advanced to 1f71ee94f2dadb743a6529ded436b266e3c93045; a read-only merge check is conflict-free, but that newer combined tree was not tested here.

Human/native/deployed-relay acceptance and full-process restart recovery remain unverified. No broad local suites or live writes were performed. The full diff, PR description and attached screenshots passed the internal-information/artifact check.

Comment thread src/features/messages/MessageManagement.tsx
Carl and others added 3 commits September 28, 2026 12:39
Preserve mention identity, attachment metadata, drafts, focus and operation recovery across channel, thread, DM, media and session surfaces. Keep message read/unread controls in a separate stacked change. Accept preserved attachment metadata at the real signing boundary and cover caption saves through the production broker.

Co-authored-by: Mongo <b07265ca2fbc3aca5c5a02ac4c5bb4532101401eaf44a591e995581c8cb167d8@agents.invalid>
Co-authored-by: Princess Donut <f0d7f043ab0904a57f19ea7215f4cd1a50ee2798a07f509eb4e9ecd275e2fe51@agents.invalid>
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>

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

Star Lord automated source review (via Wes’s account)

No new actionable source findings in this follow-up. The prior viewer/confirmation keyboard-ownership finding is addressed in the reviewed source.

  • Head: 91ec1bcace856d21ace7f36af9046772c0dba088
  • Base: a7b45d346d50ca7137a4f696239fe120dc1633da
  • Scope: the repair since previously reviewed d96d7dbf2e9652d2a83ca572929649a75f328daf: eight production lines in useModalBoundary.ts and the added browser regression. This is not a fresh audit of the entire feature or stacked #352.

useModalBoundary.ts:36–44 now returns before the viewer’s Escape/Tab handling when the event originates in a separate portalled modal. The shared AlertDialog supplies aria-modal="true" and retains Base UI keyboard/focus ownership. I traced both hook callers (MediaReviewViewer and MediaAttachment); events inside their own viewer still reach the existing dismissal/tab behavior. The new journey at tests/browser/message-management.spec.mjs:285–370 checks forward/backward wrapping, both cancellation paths, preserved viewer/comment and returned focus, then viewer dismissal and opener focus. No existing cases were removed in this repair.

Validation limits: source-only; no tests, builds, app launch or PR code execution. Pinned source blobs were hash-verified; no dirty working-tree source was used. The PR reports fail-before in Chromium/WebKit and 22/22 affected browser cases at this head; I did not independently execute those checks. Native/deployed-relay/human acceptance and successful media-comment-deletion focus remain unverified.

Hosted CI is not green: one read-only snapshot showed JavaScript failed, with two WebKit shards still in progress. The failure annotation only reports exit code 1; its cause was not established in this scoped review. DCO passed. This COMMENT is a source-review disposition, not approval, dismissal of historical reviews, or merge authorization.

Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
@morgmart
morgmart force-pushed the morganm/message-management branch from 91ec1bc to e10ebf7 Compare September 28, 2026 19:43
Carl added 2 commits September 28, 2026 12:48
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>
Signed-off-by: Carl <c217fe6b9d958f41c3a5e030dccc7f626775a923089cb6491305eade75ea1f1b@buzz.block.builderlab.xyz>

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

Star Lord automated source review (via Wes’s account)

No new actionable source findings in this focused follow-up. The rapid-edit ordering repair is consistent with its stated retained-local-history boundary.

  • Head: 7efd64b34ed29cdb44aef43ffc890ad457ef9923
  • Base: 1f71ee94f2dadb743a6529ded436b266e3c93045
  • Scope: the three-file delta since previously covered e10ebf7ecf5538825eee55273e447b45b2dd4833: outbox timestamp ordering, its deterministic regressions, and the compatibility fixture’s readiness wait. This is not a new audit of the entire message-management feature or stacked #352.

outbox.ts:667–689 advances each new edit beyond retained edits for the same viewer/channel/target, including restored, queued and confirmed records. Hydration and excessive clock-lead rejection occur before event identity, insertion or scheduling. The existing synchronous notification rebuild makes a subsequent submission see the previous intent; retries still reuse the original event. The editor catch path (useMessageEdit.ts:115–126) preserves the input when admission throws. The fold’s signed-second/ID ordering remains unchanged (fold.ts:252–257).

The added tests cover fixed-clock accepted edits and fresh-session replay, restored/queued ordering, hydration rejection, target/channel isolation and rollback rejection without insertion. These assertions belong at the shared outbox/session layer; the compatibility test retains its attachment assertions and only waits for readiness. No browser cases are added or removed by this repair. The bounded confirmed journal does not establish global ordering across unseen or evicted history, and this review does not expand that guarantee.

Validation limits: source-only, using hash-verified pinned blobs with no dirty working-tree inputs. No PR code, tests, builds or app workflows were executed. Reported local/browser passes remain author evidence, not independently reproduced results. Native GUI, deployed-relay/human acceptance and full-process restart recovery remain unverified.

One read-only snapshot of CI run 36475296286, associated with this head/base, showed JavaScript, Rust/tool integration, browser measurements and two Chromium shards passing; four browser shards were still running. DCO and security checks passed; Windows native validation was skipped. Final CI remains unresolved. This COMMENT is not approval, dismissal of historical reviews, or merge authorization.

@wesbillman
wesbillman merged commit 7a008c3 into main Sep 28, 2026
14 checks passed
@wesbillman
wesbillman deleted the morganm/message-management branch September 28, 2026 20:15
loganj pushed a commit that referenced this pull request Sep 28, 2026
#189 added a page error collector while #345 made watchPageErrors the
only allowed collector. Both merged, so main fails the Biome
page-errors rule.

Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
baxen pushed a commit that referenced this pull request Sep 29, 2026
Main now batches joined channels into shared live routes (#359) and
renews them make-before-break with replace(). A plugin kind change now
uses the same path: established channel and batch routes renew
live-only behind their current wire, which closes once the renewal is
established, and a route still replaying restarts. Live-only renewals
send limit 0, so enabling a plugin never replays old traffic or inflates
the replay count; history arrives on the next load as documented.

Also keep plugin rows out of the message management menu (#189) and
the "mark through" target in the unread badge.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: murderbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@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.

2 participants