Repository navigation
NEXUS-4082: Keep the new-message boundary stable during a visit - #2083
Conversation
6f2edb5 to
8e70de2
Compare
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 Four unread-boundary capture issues: history gating, late evidence, entry reconciliation, and post-entry arrivals. See inline evidence and validation limits.
8e70de2 to
42479be
Compare
|
Disposition of the four-point review at 42479be:
All four threads have attributed answers and invite reopening. Focused tests passed 170/170 and the boundary browser file passed 4/4 on source matching this head; hosted CI is still running. — Posted by Codex (gpt-6.1-sol) on behalf of kylez |
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.
Reviewed head 42479be7fb243288128be3bbee774afddda4dffc against base c253fc051e1aa7acb4c6c7f2f64f8b779e2a41cd.
One existing P2 remains source-confirmed: the same-visit reveal/remount finding. ConversationTab.tsx:101–106,194 changes holding while resolving a new reveal; ChannelBody.tsx:114–125 replaces the timeline with a placeholder while the outer visit provider remains mounted. The boundary and entry IDs live in Timeline state/refs, so remounting can move or remove the already-established divider within that same visit. Preserve those values with the visit owner across temporary reader unmounts and cover that transition. Linking the existing report instead of duplicating its inline comment.
One additional P3 is inline: synchronized reads can bypass the local revision fence before capture. This affects the entry annotation, not stored read state. The documented omissions for incomplete history, unloaded entry rows and local revision changes are deliberate limits, not additional findings.
Source review covered the changed files, callers/producers, visit/error paths, virtualizer anchoring, accessibility/copy behavior, tests, public description and all four attached synthetic screenshots. No concrete public-material disclosure found. The two added browser journeys have appropriate geometry/native-selection and broker-to-React integration coverage; no browser cases were removed.
Validation: no code execution, tests or app launches by this review. Existing hosted CI at this head reports required checks successful and Windows validation skipped. Inspected logs show 74 passing mounted restore cases and 20/20 repetitions of the two boundary journeys in each engine. Test counts, job elapsed time, summed execution and slowest-file evidence were inspected; no comparable base timing establishes the incremental cost. Native WebView, live multi-device ordering, and the remount scenario remain unexercised by us. This is a COMMENT review, not approval.
42479be to
7a47c48
Compare
|
Both findings in the source review are fixed at 7a47c48:
All five new mounted regressions failed on the prior head and pass after repair. Blox passed 289 focused tests, touched-file lint, and the full two-engine browser file (4/4) on source matching this commit. Hosted checks are still running. The remount and synchronized-read orderings are mounted React proof, not live native/multi-device reproductions. The deliberately retained C1 omission is unchanged; the PR describes it separately. Both threads now have attributed answers and invite reopening. — Posted by Codex (gpt-6.1-sol) on behalf of kylez |
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 One test-maintenance finding; no new production correctness or security defect established in this pass.
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.
Reviewed head 7a47c48a6c69f041f84d32ff45d5fbd631c7cd82 against base c014f54b33f6003612c3b1f2e6314354bdcb514d, focusing on changes since reviewed head 42479be7fb243288128be3bbee774afddda4dffc.
No new actionable findings. Both previous production concerns are addressed in source: completed/empty boundaries and pending baselines survive temporary timeline unmounts under the existing visit token, and changes to already-known entry attention invalidate pending capture without depending on the local read-intent revision. The five added mounted cases cover these orderings. The documented conservative omissions remain intentional; this review does not establish stronger guarantees for evidence that was never known at entry.
The existing test-fixture cleanup comment remains separate optional maintenance; it is not duplicated here. The pending attention scan does more eager work; its latency impact was not measured. No browser journeys were added or removed in this follow-up.
Inspected the public description and all four labeled synthetic screenshots; no concrete disclosure found. Source-visible separator semantics and error/retry gates remain intact. The screenshots represent 993af5f, not a fresh render of this head.
Validation: source-only; no tests, builds or app execution by this review. A one-time hosted-check snapshot reports CI required successful and Windows validation skipped. The author’s detailed test counts were not independently reproduced. Native WebView behavior, live multi-device ordering, current-head focus/scroll behavior and performance remain unverified by us. This is a COMMENT review, not approval.
7a47c48 to
7abf46d
Compare
|
The test-maintenance review is addressed in 7abf46d: obsolete latest-message fixture bookkeeping and one equivalent invocation were removed, retaining no-divider/unchanged-unread assertions and the browser readiness poll. The whole touched restore file passes on Blox (78/78); Biome and the narrow independent delta review are clean. Hosted CI is pending. On-thread disposition. — Posted by Codex (gpt-6.1-sol) on behalf of kylez |
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.
Reviewed head 7abf46d37cf0eb671b3e0be94558ae32559bea56 against base c014f54b33f6003612c3b1f2e6314354bdcb514d, focusing on the fixture cleanup since reviewed head 7a47c48a6c69f041f84d32ff45d5fbd631c7cd82.
No new actionable findings. The endpoint comparison changes only ChannelTimeline.restore.test.tsx (+13/-38); production and browser files are unchanged. The removed latestMessage fixture field is not consumed by the boundary path. Both former unloaded-entry variants keep read-sync incomplete until after the late row appears, so neither exercises the empty-row/count branch; the retained case preserves the no-separator and still-unread assertions. Deferred entry/snapshot ordering and fixture teardown remain intact. No browser cases were added or removed in this follow-up.
Inspected the updated public description and all four synthetic screenshots; no concrete disclosure found. The screenshots remain from 993af5f, not a new rendering of this head.
Validation: source-only; no tests, builds or app execution by this review. A one-time current-head CI snapshot showed both Vitest flake jobs successful, broader JavaScript/browser/native checks still running, and Windows validation skipped. The author’s reported 78-case Blox pass was not independently reproduced. Completed flake-job wall times were 66s and 55s, but detailed counts, summed execution and slowest-file evidence were unavailable from the guarded log reads. No measured speedup is claimed; native WebView, live multi-device ordering and current-head focus/scroll behavior remain unverified by us. This is a COMMENT review, not approval.
7abf46d to
7882e7f
Compare
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.
Reviewed head 7882e7f against base 7878f06, focusing on the actual endpoint changes since 7abf46d.
No new actionable findings. Moving entry membership, the read-intent revision and known attention into the existing visit owner addresses delayed first timeline mounting: loading/seek arrivals cannot become entry membership, and known read-state changes preserve the conservative omission policy. The added mounted cases cover these transitions; the existing browser journey now re-enters loaded history before testing read-sync recovery. No browser cases were added or removed in this follow-up.
Source-only review: no tests, builds, app execution, native focus checks, live multi-device validation or latency measurements were performed by this review. The PR’s reported Blox runs are author-provided evidence; the single hosted-CI snapshot still had most jobs running. Cold-entry/unknown-evidence omissions remain documented limitations. Public description and all four attached synthetic screenshots were inspected. This is a COMMENT review, not approval or merge authorization.
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 One merge-blocking test regression at this head; no new production or security defect established.
7882e7f to
b5b7ea3
Compare
|
The fixture regression in the review is fixed in b5b7ea3. All ten affected files pass on Blox (516/516) with matching source hashes and clean touched-file Biome. Production code, assertions and test counts are unchanged; hosted CI is still running. The on-thread reply gives the fixture and inactive-capture disposition. — Posted by Codex (gpt-6.1-sol) on behalf of kylez |
f44805e to
13b9e29
Compare
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.
No new actionable findings. Reviewed head 13b9e29ad227ee8cb78e5fa4c806a02f045d4ed9 against supplied base f1e40ea0e559db97f6ec6815b92d13a6a861d377 (PR merge-base 44aa6ab34833043c94bb8e78d7cceeb8d59fbe4a), separating inherited changes from the follow-up since 8abe32d0e922cfc52f5ddb7f0ab4dcffb4de8440.
The source retains provisional and unresolved entry evidence across delayed mounting and same-visit remounts, so later synchronized reads cannot substitute a newer unread boundary. Saved-boundary restoration now keeps that row mounted before measuring its divider inset. The mounted ordering cases and added warm-cache browser fixture cover these seams in source; the latter toggles timeline presentation rather than exercising the actual inline-seek route. The separate opening-focus behavior and its dedicated tests were withdrawn, as documented; the prior focus finding therefore no longer applies to this divider-only change.
Reviewed the public description and all four attached synthetic screenshots, with no concrete disclosure finding. The screenshots are from 993af5f, not a fresh render of this head. Existing loading/error/retry paths and selection-copy filtering were source-traced. Conservative omission when entry evidence cannot establish a boundary remains an intentional limit.
Validation: source-only; no tests, builds or app execution by this review, and current CI was not assessed. The author’s reported runs were not independently reproduced. Native WebView, live multi-device ordering, actual focus/scroll behavior, accessibility and latency remain unverified by us. The referenced issue’s contents were not independently retrieved; scope was assessed from the PR and repository documents. This is a COMMENT review, not approval.
c24a9d9 to
5817745
Compare
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated follow-up source review via Wes’s account, with independent Rocket, Mantis and Groot coverage.
No new actionable findings. Reviewed head 58177457c49d90f98c3b76538ddef73e22c4da38 against supplied base cbc71c85afc89abbb5120a3a584649231c26e91d (PR merge-base 6565eea00b0a875a0b4ebf9766ce915069de44f1), focusing on changes since reviewed head 13b9e29ad227ee8cb78e5fa4c806a02f045d4ed9 and separating inherited base changes.
Entry-history completeness now remains fixed at visit entry; later paging cannot establish a boundary from incomplete entry evidence. Backdated arrivals before first timeline mounting are skipped without discarding a known entry divider. Both boundary subscriptions stop for inactive or failed visits and after capture. The mounted regression cases cover these transitions. The integration retains chronological entry scanning and combines the divider inset with contained-message restoration without adding the offset twice.
The documented omission for incomplete history or uncertain entry evidence is intentional. The existing browser boundary spec is byte-identical to the prior reviewed revision; no browser scenarios were added or removed in this follow-up. Reviewed the updated public description and all four synthetic screenshots, with no concrete disclosure finding. Those screenshots remain from 993af5f, not a fresh rendering of this head.
Source-only: no tests, builds or app execution by this review; current hosted CI was not assessed, and the author’s reported Blox runs were not independently reproduced. Native WebView, live multi-device ordering, current-head focus/scroll behavior and latency remain unverified. The referenced issue’s contents were not independently retrieved; scope was assessed from the PR and repository documents. This is a COMMENT review, not approval.
5817745 to
5a39bb3
Compare
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Kyle Zemek <kylez@squareup.com>
5a39bb3 to
8219ae7
Compare
…lled-read * origin/main: (28 commits) Run native Codex through plugin execution targets (#2150) test: split Solo access browser scenarios (#2499) feat(agents): let any agent change Buzz settings through the MCP (#2248) Let plugins decorate host sidebar rows and add sort modes (#1853) NEXUS-4082: Keep the new-message boundary stable during a visit (#2083) feat(agents): move version history into settings (#2374) fix(agents): read goose Activity by saved session ids and render memory bodies on open (#2466) feat(channels): starter channels: admin switch and relay auto_join flag (#1624) fix(plugins): read Square HTTPS sources with the Block SSH agent (#2465) feat(plugins): let agents request plugin changes for owner review (#2280) fix(messages): keep the hover toolbar while its menu is open (#2439) feat(workflows): suggest channel members for @ in Send Message text (#2309) AIDA-2344: Exclude muted channels from Dock (#2313) feat(search): add slack-style message search operators on desktop (#2463) test: fix invitation and coordination checks on main (#2517) perf(search): name searched people from the profile search response (#2353) Use the merge queue for pull requests that require it (#2452) fix(browser): load scheme-less address input over https, falling back to http (#2467) fix(identity): stop one-click key unlinks and show the linked backed-up key (#2486) fix(test): observe coordination transition events (#2297) ... Signed-off-by: Gil Birman <gil@squareup.com> Co-authored-by: Claude <noreply@anthropic.com> # Conflicts: # docs/buzz-help.md
…ettings-shift-esc * upstream/main: (91 commits) test(browser): click timeline rows only after the timeline settles (block#2560) ci: split Vitest across six shards so 3/4 stops hitting the job cap (block#2571) feat(github): unfurl PR and issue links inline with their title (block#1693) fix(auth): style federated sign-in callback pages (block#2549) fix(agents): align creation model controls with identity (block#2480) Run native Codex through plugin execution targets (block#2150) test: split Solo access browser scenarios (block#2499) feat(agents): let any agent change Buzz settings through the MCP (block#2248) Let plugins decorate host sidebar rows and add sort modes (block#1853) NEXUS-4082: Keep the new-message boundary stable during a visit (block#2083) feat(agents): move version history into settings (block#2374) fix(agents): read goose Activity by saved session ids and render memory bodies on open (block#2466) feat(channels): starter channels: admin switch and relay auto_join flag (block#1624) fix(plugins): read Square HTTPS sources with the Block SSH agent (block#2465) feat(plugins): let agents request plugin changes for owner review (block#2280) fix(messages): keep the hover toolbar while its menu is open (block#2439) feat(workflows): suggest channel members for @ in Send Message text (block#2309) AIDA-2344: Exclude muted channels from Dock (block#2313) feat(search): add slack-style message search operators on desktop (block#2463) test: fix invitation and coordination checks on main (block#2517) ... Signed-off-by: Cheng Peng <cheng@block.xyz> Co-authored-by: Cursor <cursoragent@cursor.com>
* upstream/main: (22 commits) test(browser): click timeline rows only after the timeline settles (block#2560) ci: split Vitest across six shards so 3/4 stops hitting the job cap (block#2571) feat(github): unfurl PR and issue links inline with their title (block#1693) fix(auth): style federated sign-in callback pages (block#2549) fix(agents): align creation model controls with identity (block#2480) Run native Codex through plugin execution targets (block#2150) test: split Solo access browser scenarios (block#2499) feat(agents): let any agent change Buzz settings through the MCP (block#2248) Let plugins decorate host sidebar rows and add sort modes (block#1853) NEXUS-4082: Keep the new-message boundary stable during a visit (block#2083) feat(agents): move version history into settings (block#2374) fix(agents): read goose Activity by saved session ids and render memory bodies on open (block#2466) feat(channels): starter channels: admin switch and relay auto_join flag (block#1624) fix(plugins): read Square HTTPS sources with the Block SSH agent (block#2465) feat(plugins): let agents request plugin changes for owner review (block#2280) fix(messages): keep the hover toolbar while its menu is open (block#2439) feat(workflows): suggest channel members for @ in Send Message text (block#2309) AIDA-2344: Exclude muted channels from Dock (block#2313) feat(search): add slack-style message search operators on desktop (block#2463) test: fix invitation and coordination checks on main (block#2517) ... Signed-off-by: Cheng Peng <cheng@block.xyz> Co-authored-by: Cursor <cursoragent@cursor.com>
* upstream/main: (91 commits) test(browser): click timeline rows only after the timeline settles (block#2560) ci: split Vitest across six shards so 3/4 stops hitting the job cap (block#2571) feat(github): unfurl PR and issue links inline with their title (block#1693) fix(auth): style federated sign-in callback pages (block#2549) fix(agents): align creation model controls with identity (block#2480) Run native Codex through plugin execution targets (block#2150) test: split Solo access browser scenarios (block#2499) feat(agents): let any agent change Buzz settings through the MCP (block#2248) Let plugins decorate host sidebar rows and add sort modes (block#1853) NEXUS-4082: Keep the new-message boundary stable during a visit (block#2083) feat(agents): move version history into settings (block#2374) fix(agents): read goose Activity by saved session ids and render memory bodies on open (block#2466) feat(channels): starter channels: admin switch and relay auto_join flag (block#1624) fix(plugins): read Square HTTPS sources with the Block SSH agent (block#2465) feat(plugins): let agents request plugin changes for owner review (block#2280) fix(messages): keep the hover toolbar while its menu is open (block#2439) feat(workflows): suggest channel members for @ in Send Message text (block#2309) AIDA-2344: Exclude muted channels from Dock (block#2313) feat(search): add slack-style message search operators on desktop (block#2463) test: fix invitation and coordination checks on main (block#2517) ... Signed-off-by: Cheng Peng <cheng@block.xyz> Co-authored-by: Cursor <cursoragent@cursor.com>
…nks-in-browser * origin/main: (138 commits) feat(composer): optional ⌘/Ctrl+Enter to send (block#2331) test: use keyboard activation for reply caret setup (block#2557) test: settle member search before invitation expansion (block#2532) test(agent-command): isolate the copied executable writer (block#2537) test(sidebar): wait for the rolled-back row to leave Starred before collapsing Work (block#2574) test(browser): click timeline rows only after the timeline settles (block#2560) ci: split Vitest across six shards so 3/4 stops hitting the job cap (block#2571) feat(github): unfurl PR and issue links inline with their title (block#1693) fix(auth): style federated sign-in callback pages (block#2549) fix(agents): align creation model controls with identity (block#2480) Run native Codex through plugin execution targets (block#2150) test: split Solo access browser scenarios (block#2499) feat(agents): let any agent change Buzz settings through the MCP (block#2248) Let plugins decorate host sidebar rows and add sort modes (block#1853) NEXUS-4082: Keep the new-message boundary stable during a visit (block#2083) feat(agents): move version history into settings (block#2374) fix(agents): read goose Activity by saved session ids and render memory bodies on open (block#2466) feat(channels): starter channels: admin switch and relay auto_join flag (block#1624) fix(plugins): read Square HTTPS sources with the Block SSH agent (block#2465) feat(plugins): let agents request plugin changes for owner review (block#2280) ... # Conflicts: # src/bundled/github/README.md
* commit 'a39dba132^': feat(github): unfurl PR and issue links inline with their title (block#1693) fix(auth): style federated sign-in callback pages (block#2549) fix(agents): align creation model controls with identity (block#2480) Run native Codex through plugin execution targets (block#2150) test: split Solo access browser scenarios (block#2499) feat(agents): let any agent change Buzz settings through the MCP (block#2248) Let plugins decorate host sidebar rows and add sort modes (block#1853) NEXUS-4082: Keep the new-message boundary stable during a visit (block#2083) feat(agents): move version history into settings (block#2374) fix(agents): read goose Activity by saved session ids and render memory bodies on open (block#2466) feat(channels): starter channels: admin switch and relay auto_join flag (block#1624) fix(plugins): read Square HTTPS sources with the Block SSH agent (block#2465) feat(plugins): let agents request plugin changes for owner review (block#2280) fix(messages): keep the hover toolbar while its menu is open (block#2439) Signed-off-by: damienrj <damien@squareup.com>
TL;DR
Keep the “New messages” divider fixed during a channel visit, even as reading advances unread state or the timeline temporarily remounts. Capture one verified entry boundary rather than deriving it from the moving unread cursor. Fixes NEXUS-4082.
How
The existing message-visit owner records loaded row IDs, verified history completeness, the read-intent revision, and read evidence before the timeline mounts. Unresolved attention stays uncertain; later synchronization cannot redefine entry. The timeline captures one unread message ID and keeps it under that visit token. It checks evidence in chronological order only through that first unread row. Messages received during loading or an inline seek cannot become the entry boundary. If the entry boundary cannot be proved, omit the divider rather than guess.
Read eligibility, focus requirements, membership checks, settled-reading dwell, and read-state writes remain with their existing owners. No new focus handoff or force-read operation is included. Normal guest-channel opening focus and Search Buzz dismissal belong to the human-owned #2140; this PR neither implements nor depends on that change.
Verification
Snapshot
8219ae795includes the merged selector repair #2493. Blox passed root typecheck, touched-file Biome, design checks, and 830 tests across 16 complete affected files. All 58 browser cases passed in Chromium and WebKit across the completenew-message-boundary,history-loading,activity-groups,activity-lifecycle, andnavigation-row-menufiles, without skips or retries. All 39 selected source hashes match the uploaded snapshot and committed files; downloaded artifact digests match the receipt. The box was deleted. Current-head hosted CI is pending.The six later-row ordering cases failed before this repair, while six earlier-row controls passed. All 12 now pass: later unresolved evidence cannot suppress an earlier proven unread boundary. Unresolved or changed evidence at or before that boundary still prevents choosing it.
The retained divider scope adds three browser scenarios (six engine cases), removing none. Browser-only coverage exercises real virtualizer geometry, keyboard scrolling, selection/copy, and signed delivery through the broker/store. The saved-offset regression previously failed with 32px drift in both engines; the original
<4pxtolerance is unchanged. The mounted ordering matrix stays in React Testing Library. No browser cases were added or removed by this repair.The remount fixture toggles timeline presentation under
MessageManagement; it does not drive the realConversationTab→ChannelBodyinline-seek route. That wiring was source-traced only. Native WebView and live multi-device behavior remain unproven.Earlier opening-focus scope was withdrawn: its ten mounted cases and two width-based browser journeys (four engine cases) were removed, with
ChannelsPage.tsxandchannel-permissions.spec.mjsrestored to base. Those cases covered the withdrawn focus behavior, not this divider. No replacement focus proof is claimed.Historical synthetic screenshots at
993af5f(not current-head native verification).Before the boundary.

After the boundary.

After channel revisit, with grouped messages.

Narrow, dark view with larger text.

Limits
The divider stays absent for the whole visit when history was incomplete or unverified at entry, even if older rows load later. It may also be absent if entry membership or evidence at or before the first unread row cannot be proved. Read-intent changes or changed evidence needed to locate the boundary before capture conservatively omit it. Uncertainty in a later row no longer suppresses an earlier proven unread boundary. A visible read/unread transition cannot exclude older unread messages, and delayed rows cannot establish entry membership.
A read change in another channel while entry awaits evidence can also suppress the divider for this visit. Entry reconciliation can clear a prior message-force marker and advance the same global revision. The public API does not expose a target-specific entry set or mutation cause. This PR retains that conservative guard; focused tests preserve the omission and verify the affected messages remain unread.
The original live #announcements unread report remains unproved. Private screenshots did not establish its channel role or focus state and are not published. This divider-only change claims no confirmed fix for that incident. Native WebView was not exercised.
Coordination
Based on main, including the merged held-reader resize and fixture repairs. Keeps the existing unread/relay owners and does not expand their APIs. The opening-focus overlap was removed rather than creating a competing owner or an ordering dependency on #2140.
— Generated with Codex (gpt-6.1-sol) on behalf of kylez.