Show exact Inbox conversations with read recovery - #499
Conversation
wesbillman
left a comment
There was a problem hiding this comment.
Changes needed: three P2 comments below cover cancelled-read error handling and keyboard focus during incomplete-preview/retry recovery.
Star Lord’s automated source review via Wes’s account. Reviewed head 0ec7b0ce5b68d259a2f6b1728a0f2a120cbb9b4a against immediate base f51741dd4ce68d07912143d22f3f6d32bee62768.
Source-only: no tests or app execution. The hosted JavaScript run failed because the inherited inbox-feed.test.ts events-budget case timed out at 5s; causal attribution is unverified. Human/live/native acceptance remains outstanding. This is a non-blocking comment review, not approval.
wesbillman
left a comment
There was a problem hiding this comment.
The three P2s in the prior review remain: pending-read cancellation reports false expiry, incomplete previews lack keyboard entry focus, and successful Retry loses focus. Those implementations are unchanged; no new findings in the inherited thread-reopen synchronization delta.
Star Lord automated source review via Wes’s account: head dd757bdd5222fd5eafbab3516c82ac1259f5c16d, base 1821f3bfcdc7666d78b72dbb9c20bcd0d77fa229.
Source-only; no code/tests/app executed. Current-head CI failed WebKit Members focus and the Rust reads_rpc_and_reports_exit_failure test; JavaScript was cancelled, with causation unestablished. Native/live/human acceptance remains unverified; not approval or merge readiness.
wesbillman
left a comment
There was a problem hiding this comment.
No further changes requested: the three prior P2s are addressed in source—pending-read cancellation, incomplete-preview keyboard entry, and Retry focus ownership.
Star Lord’s automated source review via Wes’s account: head 3e9dce0b0141a9005a63146d0210ac8e05c745e0, immediate base 98cd54013a32c86ea6c98cbe200a61bd8b02fa7b.
Source-only follow-up; no tests or app execution, and native/live/human acceptance remains unverified. The current-head CI snapshot has a Chromium crowded-tab failure (causation unestablished), other jobs still running and Windows skipped; this is not approval or launch-batch readiness.
wesbillman
left a comment
There was a problem hiding this comment.
No further changes requested: the incremental Inbox diff against #495 is unchanged from the previous review, and the retained cancellation/focus fixes plus relevant inherited UI integration show no new actionable defect.
Star Lord automated source review via Wes’s account — head 588d4db981dd43c24b38fb58b200f613a2bb45d9; immediate base e25c7d784562a169bb1fa113f626c5d432597b94.
Source-only; no tests or app execution, browser CI was still running at the one-time snapshot, and human/live/native/packaged acceptance remains outstanding. This is not approval or launch-readiness evidence.
wesbillman
left a comment
There was a problem hiding this comment.
No additional changes requested in this reconciliation follow-up. The 17-path incremental Inbox change retains its previously reviewed additions/removals; the cancellation and focus repairs and inherited startup/reveal and Send to channel wiring remain intact in source.
Star Lord automated source review via Wes’s account — head 1be6b74943aa6eda9da158c7ce3f4a383a02e2bc, base f264c623887717b7fd48c4af39b0c719e0104256.
No tests or app/live/native/package workflows run; browser journeys were still running in the one-time CI snapshot, and Inbox/Drafts launch acceptance remains outstanding—not merge authorization.
1be6b74 to
c52bd87
Compare
wesbillman
left a comment
There was a problem hiding this comment.
No additional changes requested in this linear-stack follow-up. All 17 Inbox files match the previously reviewed revision; the inherited hosted-community/broker changes introduce no new Inbox integration defect identified in source.
Star Lord automated source review via Wes’s account — head c52bd87c8c8d9a73d0722b3c6330c64f12f05d39, base f869df84ed24e3ca176e978c7476c0d0e088a44a.
Source-only: no tests or app workflows executed; CI not assessed, and human/live/native/packaged Inbox–Drafts acceptance remains unverified—not approval or merge authorization.
c52bd87 to
db2d0aa
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Changes needed: [P2] Replace the appended Inbox screenshot with fixture-only or irreversibly redacted content. This public PR now exposes a readable private DM, other people’s names/avatars, private-channel names and an internal design-document link. Remove the live-workspace capture (b352f855-…) and replace it with a synthetic Inbox example that preserves the UI demonstration without the conversation or workspace data.
No additional code findings in this follow-up: the 17 Inbox paths retain the reviewed source, and the added browser focus/value barriers preserve the existing assertions.
Star Lord automated source review via Wes’s account — head db2d0aaa8d7e5847e5fb8a173efcaaaa2992ec6d; base c9946be433f1957713a19cffca614947c17514ee. Source and the single attached image inspected; no tests/app executed. CI was still running at one snapshot, Windows skipped; human/live/native acceptance remains unverified.
wesbillman
left a comment
There was a problem hiding this comment.
Changes requested: address the three P2 inline findings and add regression coverage for those boundaries before merge.
Publication blocker: replace the PR screenshot with synthetic fixture evidence; it exposes private workspace content. Arrange removal of the original attachment and retained history copies through the appropriate repository/platform controls, rather than only replacing the displayed image. The existing inbox-light-1280.png fixture capture is a suitable source.
Carl, an automated reviewer, commenting via Wes’s GitHub account. Reviewed head db2d0aaa8d7e5847e5fb8a173efcaaaa2992ec6d against base c9946be433f1957713a19cffca614947c17514ee.
Validation: targeted DM reading probes reproduced the defect in Chromium and WebKit; a mounted real-session reconnect probe and a Chromium layout probe established the other findings. These ran at c52bd87c; the relevant implementation paths and original 17-path Inbox patch are unchanged at the reviewed head. I reviewed the intervening source delta, including the sidebar test's focus/value synchronization repair. Current-head CI was still running at the snapshot. Prior-head CI passed all 18 Inbox executions but failed one WebKit sidebar case; the new repair is not yet hosted-CI clearance. No full local suite, live-relay, human or native acceptance claimed.
db2d0aa to
3d81709
Compare
3d81709 to
307218e
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Bounded re-review at 307218ee: the DM reading scope, reader retention/outside-focus behavior, container-width layout, and two optional list fixes are addressed. Two P2 defects remain in the revalidation repair, detailed inline: lost in-detail keyboard focus and continued playback from withheld content. I recommend fixing these before merge.
The new reading/revalidation browser files passed 4/4 in Chromium/WebKit at 3d81709a; additional real-broker probes reproduced both remaining boundaries. The final rebase to 307218ee changes only two upstream Pi-model paths; all reviewed/tested frontend paths are identical. New-head hosted CI is still pending. Native/live and human acceptance remain unverified.
The screenshot is removed from the current description; original attachment/history cleanup remains explicitly unconfirmed and still needs repository/platform support. This is not an approval or merge authorization.
307218e to
8ce1384
Compare
wesbillman
left a comment
There was a problem hiding this comment.
No additional code changes requested. The two P2 findings in the previous review are addressed in source: guarded focus handoff and pausing withheld audio/video without recovery replay; no new material defect found in the related modal/action-bar integration.
Star Lord’s automated source review via Wes; head 8ce13846c90c19e47d84b18edc4dfe14eb72b4a9, base 8eca3958a2210cfd09e9a3e02e2b0aa83e66718c. No code/tests executed; reported browser results, native/live behavior and human acceptance remain independently unverified, and the 18:41 UTC CI snapshot still had running browser shards. Original screenshot/attachment-history cleanup remains unverified as previously reported; this is not approval or merge authorization.
Preserve exact targets, captured read retries, cancellation and keyboard recovery. Share DM composer reading, retain admitted readers during incomplete revalidation while retiring their transient overlays, and collapse split panes by available width. Keep report operations and shared modal focus safe across suspension and recovery. Cover the boundaries with focused owner and browser regressions. The inherited sidebar test waits for its dialog focus handoff and verifies the entered value; no test budgets or retries change. Signed-off-by: tulsi <tulsi@block.xyz>
8ce1384 to
1fd5b83
Compare
wesbillman
left a comment
There was a problem hiding this comment.
No new code changes requested: the five-file upstream bot-role delta preserves explicit confirmation, fresh authority checks and readback-only recovery; the reviewed Inbox implementation is unchanged. Please confirm repository/platform cleanup of the original attachment/history under the existing publication finding; the current description has no image, but historical removal remains unverified.
Star Lord’s automated source review via Wes — head 1fd5b8348418d730cb56b3108682147aa69c1657, base 0e20a7824e4d99b9b5b1285aac1ff75b8b5fe32a. Source-only; no code/tests executed, browser journeys still running at the 19:01 UTC snapshot, and live/native/human acceptance unverified; not approval.
Latest atomic stack rebase — 2026-10-02 14:51 EDT
Head
1fd5b834on main0e20a782. Both refs were published together withgit push --atomicand explicit old-SHA leases, after normal hook validation in each owning worktree. Both are conflict-free and retain one incremental commit per PR. Existing ready-for-review state was preserved; formal approvals remain required.Rebased onto upstream #540 (explicit bot admin promotion). Verified both feature patch IDs are unchanged and each head's tree delta is exactly the five-file upstream change. No Inbox/Drafts source or coverage changed. Both previous heads had terminal-green CI/DCO; those results do not certify this new head. Fresh exact-head CI is running and monitored. The local notification-clock patch remains excluded. Earlier head/check sections below are historical.
Latest focused reconnect repairs — 2026-10-02
Current head
8ce13846on main8eca3958. Clean one-commit-per-layer stack. Earlier head/check sections below are historical. Both PRs remain draft for this behavior-changing follow-up; human acceptance, formal approvals and publication cleanup remain separate.Two new Wes P2 boundaries are repaired in #499 and inherited by #502:
Also fixed the related media-review CI regression: explicit return focus wins over same-commit automatic thread-header focus while respecting later user movement and unavailable openers. The reconnect journey now orders incidental read-state publication around its intentional socket outages rather than allowlisting 503 errors. The inherited profile-avatar paint case waits for real menu teardown/final focus before its outside-blur assertion; no product menu behavior changed.
Independent bounded reviews rated both repairs and the modal/test lifecycle 9/10 with no remaining blockers. Focus/media/modal owner batch passed 206/206; rebased action-bar integration passed 13/13 after updating its existing retirement test for main's deferred control mounting. New actual focus/caret/playback reconnect file passed 4/4 Chromium/WebKit and all existing media-review browser cases passed 14/14 before main integration. Final five-file integration passed 46/46 Chromium/WebKit (3.4m local wall): Inbox reconnect, Drafts, shared media review, profile-avatar paint and message actions. Its production/browser files match this head; the only later delta is the passing lazy-action-bar unit assertion. Both normal pre-push hooks passed; both remote heads are conflict-free and #502 contains one incremental commit. Fresh hosted CI is running, not yet green. No timeouts, retries, policies or assertions were relaxed.
Browser cases: one added scenario per engine, splitting the existing portal journey from focused-composer/playback to stay within the unchanged five-attempt live reconnect policy. No previous contract coverage removed; detailed failure/access/blur/media listener permutations remain in mounted owner tests. Native playback uses real decoded video and bounded generated PCM audio, with exact post-pause positions through failure/recovery. These are synthetic fixture checks, not live/native/package or human acceptance.
Main's Rust 1.98.1 pin, host attestation and deferred desktop action bars are retained. The Drafts incremental patch is unchanged; local notification-clock edits remain excluded. Original screenshot attachments/history still need repository/platform removal; no new live imagery is published.
Latest rebase — 2026-10-02 13:56 EDT
Current head
307218eeon main75568535. Rebased the clean one-commit-per-PR stack onto main75568535(#513, upstream Pi executable-fixture fix and spawn diagnostics). Both explicit-leased pushes and normal hooks passed; DCO is successful and both PRs are conflict-free. Existing ready-for-review status was preserved; this does not attest new human acceptance or reviewer approval.Verified both incremental feature patch IDs are unchanged, and each new head differs from its previous head by exactly the two-file upstream #513 change. No Inbox/Drafts implementation or coverage changed. Prior 80/80 Chromium/WebKit evidence remains evidence for the unchanged frontend; it is not a new-head native validation claim. Fresh exact-head CI is running and monitored. Local notification-clock edits remain excluded. Earlier head/status sections below are historical; publication asset/history cleanup and formal approvals remain separate.
Latest review repairs — 2026-10-02
Current head
3d81709a, one commit on main417c9b1f. Earlier head/check sections below are historical. This PR remains draft: fresh hosted CI, human try, formal review and publication cleanup are separate gates.Addressed the three new P2 code findings and two optional list fixes:
Independent follow-up reviews covered the repairs and caught the portal/report cases before this push; all identified blockers were fixed. No backend/protocol/DS rewrite, new persistence owner, timer, timeout relaxation, or test retry was added. The additional overlay repair is about 199 production lines (including its report/modal corrections); this does not shrink the overall feature diff.
Evidence: 337/337 final integrated focused tests; 78/78 shared modal/viewer tests; normal pre-push types/1,217 related unit/design checks passed. Original-code regressions fail for DM focus, selected 720px layout, reconnect reader identity, body-portalled video, pending-report lifecycle and modal StrictMode restoration. Final seven-file integration passed 80/80 Chromium/WebKit (4.6m local wall): Inbox, Drafts, native reading/portal reconnect, StrictMode, shared composer focus and exact navigation. Checked tree
59a5412ais identical before/after the message-only history amendment to final #502d25771bb; both DCO checks pass, fresh hosted CI is still pending. Two browser scenarios were added for native DM reading and real-broker reconnect/portal withholding; existing layout journey extended at 720/900/390px. No cases removed. Failure/access/persistence permutations remain in mounted/service tests. Local measurements are fixture evidence, not native/live or human acceptance.Publication: live-workspace screenshot removed from this description. Removal of the original attachment and retained history copies still requires repository/platform support; no claim they were deleted. Use synthetic fixture captures for any replacement.
Latest: rebased stack and sidebar CI setup repair (2026-10-02)
Head db2d0aa, clean linear stack on current main
c9946be4; one commit per PR, no merge commits. Earlier head sections below are historical. The prior #499 WebKit shard failed when Section name remained empty after fill during a menu/dialog handoff. The narrow test-only change waits for the same initial-focus boundary used by the preceding case and asserts the inserted value before submission. No product code, timeouts, assertions or retry settings changed. The trace does not establish the precise native lost-insertion mechanism; no claim of deterministic reproduction or flake elimination.Full navigation-group-icons file passed 4/4 Chromium/WebKit, integrated Inbox/Drafts 26/26, plus normal push hooks (types/related tests/design). Local original focused WebKit case also passed, so green-before/after alone is not causality proof. No cases added or removed. Both DCO checks pass; fresh hosted CI pending. User authorized this rebase/repair and exact-leased history updates; approvals remain separate. The separate local notification-clock patch remains excluded.
Current linear stack (2026-10-02)
Head
c52bd87c: one commit on main f869df8. Rebuilt with explicit author permission to remove already-squashed #495 ancestry and merge-commit history that blocked GitHub stack #541's rebase. Pushed with exact old-head force-with-lease checks; original refs retained locally.Verified the rebuilt tree equals the original head reconciled with current main, and the incremental stable patch ID is unchanged. No production/test content removed, no feature-scope or line-count reduction claimed. Normal sign-off and hooks passed (30 related files / 403 tests, TypeScript/design); new DCO passed, fresh CI running. The uncommitted notification-clock test edit is excluded. Earlier head/check records below are historical.
Review and merge order remains #499 → #502. Repository approvals still apply; no merge or human-acceptance attestation was performed.
Category: new-feature
User impact: Review recent DMs, mentions and participating threads, open the exact conversation, and continue inline.
Scope
Base:
tulsi/inbox-evidence(#495). Presentation consumes shared unread/feed evidence andsession.agentChoices; existing channel/thread readers, composer and outbox remain the owners. No Drafts implementation here.Wes repairs
Review footprint
Current incremental diff: production +1,469/−40, tests/fixtures +3,193/−5, docs +40/−9 (17 files). This is still a substantial feature delta; stacking does not make it a tiny change. No behavior, assertions or failure paths were removed to meet a line target. The original #422 assertion split remains documented in
docs/inbox.mdon the stack.Validation at
0ec7b0cef51741ddand maincc0c47c7; main scroll fixes retained.Browser coverage / cost
Eight browser cases (16 engine executions) against the evidence base; none removed. They prove actual app routing, exact virtualized reveal, native focus/portal dismissal, responsive geometry, IndexedDB failure recovery and broker publication. State permutations remain in RTL. This repair round extends existing cases rather than adding a new matrix. Full two-engine local run: 43.8s wall with one worker; this is correctness evidence, not a controlled hosted performance comparison. No retries or relaxed assertions.
Stack and merge procedure
main; either order.maindiff.mainand verify its incremental diff/checks before merging; repeat for Resume scoped drafts in Inbox and discard deleted attachments #502 after Show exact Inbox conversations with read recovery #499. Do not squash a child into the parent feature branch or assume GitHub retargeting alone removes ancestor commits.Review-comment follow-up, 2026-10-01
Current head 7fe0dfb, comment fixes 511a8a1, including #495 budget-test repair152595b4. All three review threads have implementation/test replies; resolution left to reviewer. Closed pending multi-step reads quietly cancel remaining steps while preserving real save/access errors; incomplete detail gets one-time focus; pending Retry retains focus and hands it off only if still owned when its alert disappears.
New controlled regressions failed before repair. Full60Inbox RTL and16Chromium/WebKit executions pass; +1browsercase perengine for narrow placeholder nativefocus, none removed; existing narrow read-recovery case now keyboard-driven. Normal push hooks:153files/2,756tests, TypeScript/design pass. Independent source review found no blocker. Returned to draft because behavior changed; no human acceptance/readiness attestation.
The prior dd757bd CI failure also included an inherited Pi fixture spawn failure and Members restoration staying at Opening Buzz. No demonstrated causal link to Inbox was found; neither is called a flake or silently fixed. New hosted run pending.
2026-10-02 upstream CI reconciliation
Current head
588d4db9includes parente25c7d78and main75e6dc39's two-shard Vitest setup, panel-tab selector repair, icon/media integration. No feature fix was dropped and no timeouts/assertions weakened. Push hooks passed 156 files / 2,900 tests, TypeScript/design. Integrated top completed all 26 Chromium/WebKit Inbox executions. Narrow integration review found no blockers; current hosted CI pending. DCO passed. All existing review threads remain resolved. Squash into main only after #495, retarget/reconcile and repeat required gates; no ancestor-branch squash.Reconciled after #495 squash merge (2026-10-02)
#495 landed on main as
f264c623. This PR now targets main, head1be6b749. Merged main without rewriting published commits; resolved only the add/adddocs/inbox.mdconflict by retaining the reviewed UI extension and common evidence contract. All 17 incremental paths retain their original added/removed lines relative to the now-squashed parent; #495 implementation/test files exactly match main, so this PR does not reintroduce evidence changes. New upstream startup/focus and Send to channel wiring is preserved.Verification: TypeScript, 205 focused tests /5 files, 18/18 Chromium/WebKit Inbox checks, and normal commit/push hooks (30 related files /403 tests plus design) pass. Independent narrow integration review found no blockers. DCO passed; fresh hosted CI running. GitHub now applies main's required review gate. Merge #499 into main next, then reconcile/retarget #502; do not squash #502 into this feature branch.