Conversation
Signed-off-by: Tom Brow <tomb@block.xyz>
* origin/main: (400 commits) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) fix(web): route mobile invite downloads to app stores (#7702) feat(mobile): show build number with version in settings (#7697) release(mobile-infra): buzz-push-gateway 0.3.0 (#7685) Add authenticated WebSocket recovery telemetry (#7546) Instrument database pool roles (#7356) refactor(desktop): share relay request construction (#7667) fix(db): allow naturally expired push lease replacement (#7653) perf(mobile): verify notification cache events in background (#7650) ... Signed-off-by: Tom Brow <tomb@block.xyz> # Conflicts: # mobile/lib/features/channels/channel_detail_page/message_bubble.dart # mobile/lib/features/channels/channel_detail_page/message_list.dart # mobile/test/features/channels/channel_detail_page_test.dart
Signed-off-by: Tom Brow <tomb@block.xyz>
|
@codex review |
🔐 Codex Security Review
|
|
@builderbot review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@buzz-security-review 110e988 |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
github-actions
Bot
added
the
codex-security-review-current
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 110e988a54
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Tom Brow <tomb@block.xyz>
github-actions
Bot
removed
the
codex-security-review-current
|
@codex review |
|
@builderbot review |
|
@buzz-security-review 24aef3e |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24aef3e6f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Tom Brow <tomb@block.xyz>
|
@codex review |
|
@builderbot review |
|
@buzz-security-review c462bd4 |
github-actions
Bot
added
the
codex-security-review-current
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c462bd42c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Tom Brow <tomb@block.xyz>
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: REQUEST CHANGES
Reviewed: 5511b56fcf0047f9d0d4887dc75125b6933a6bbd..c462bd42c4ae66dd45b1420a81e93ce4fb90c520 (exact live head c462bd42c4ae66dd45b1420a81e93ce4fb90c520)
Risk: medium — mobile gesture/navigation eligibility across optimistic-send and relay-confirmation state transitions.
Blocking finding: mobile/lib/features/channels/channel_detail_page/message_bubble.dart:33-40,128-143 derives thread tapability from pendingLocalMessagesProvider. The successful send path calls completeLocalMessage immediately after relay publish ACK (mobile/lib/features/channels/send_message_provider.dart:98-115), which removes that pending entry, while the optimistic reply intentionally remains in threadLocalRepliesProvider until authoritative EVENT/query corroboration. In the ordinary ACK-before-EVENT interval, the reply is visible but tapping its root does nothing.
Both independent lanes reproduced this by extending the new widget test through addLocalMessage → completeLocalMessage → tap: expected one ThreadDetailPage, found zero. I independently repeated that exact causal probe at clean head c462bd42; it failed, then the unmodified tree was restored clean.
Author action: Derive tapability from state that survives successful publish ACK until authoritative relay/query replacement, without retaining an empty auto-dispose overlay for every row. Add a regression covering local reply added → publish completes/ACK → no EVENT yet → root tap opens and shows the optimistic reply; retain zero-reply no-op, relay-only reply, and long-press → Reply coverage.
Verification owner: author for code and causal regression; reviewer/integration for fresh exact-head package gate and mutation proof.
Validation: integrated lanes ran just mobile-install mobile-check mobile-test / equivalent exact-head mobile gates successfully (2,259 passed, 4 skipped, plus 3 unconfigured-push tests); local full just mobile-test && just mobile-check passed on the preceding head before the final one-line state-source change. The exact-head causal production-sequence probe fails as described. GitHub Mobile and Mobile Swift jobs are still in progress; that is a gate-status confidence gap, not the blocking reason.
Manual/native evidence: no independent native iOS/VoiceOver run. Widget and source tracing establish the state-transition defect; native observation remains reviewer/release ownership.
Residual risk: Pending CI may add evidence, but cannot clear the reproduced ACK-before-EVENT defect. Any new head expires this verdict.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — REQUEST CHANGES on exact head c462bd42c4ae66dd45b1420a81e93ce4fb90c520 (base 5511b56fcf0047f9d0d4887dc75125b6933a6bbd).
[P2] Keep the optimistic thread tappable after publish ACK
_MessageBubble now derives hasLocalReplies from pendingLocalMessagesProvider (mobile/lib/features/channels/channel_detail_page/message_bubble.dart:33-40,128-143). But the normal successful-send path awaits the relay publish/OK future and immediately calls completeLocalMessage (mobile/lib/features/channels/send_message_provider.dart:97-115), which removes the reply from that pending map (mobile/lib/features/channels/channel_messages_provider.dart:297-300,336-341). The reply intentionally remains in threadLocalRepliesProvider until authoritative relay data replaces it because OK and EVENT delivery are unordered (channel_messages_provider.dart:303-341).
That leaves an ordinary ACK-before-EVENT interval where the optimistic reply is still visible but tapping its root does nothing. Long-press → Reply is not an equivalent way to open the existing visible thread.
I reproduced this at the exact head by adding notifier.completeLocalMessage('local-reply') after addLocalMessage in the PR's new focused widget test, then running:
cd mobile && flutter test test/features/channels/channel_detail_page_test.dart \
--plain-name 'a local reply makes its thread immediately tappable'
Result: expected one ThreadDetailPage, found zero; exit 1. I restored the source afterward. Two independent review lanes reproduced the same causal failure. The shipped test stops before completeLocalMessage, so it does not cover the production lifecycle transition.
Author action: derive tap availability from state that survives successful publish ACK until relay EVENT/query corroboration, without retaining empty thread overlays merely by rendering rows. Extend the regression through addLocalMessage → completeLocalMessage → tap, asserting that the thread opens and shows the optimistic reply. Retain the zero-reply tap no-op and long-press → Reply coverage.
Verification owner: author for fix and causal regression; :bot: Jude’s code review agent for fresh-head rerun and mutation check.
Other validation
- Loaded replies and positive relay-only summaries remain tappable; zero/absent summaries disable ordinary tap; long-press Reply remains independent.
- Independent exact-head full mobile gate passed:
just mobile-install mobile-check mobile-test(2259passed,4skipped; unconfigured-push suite3passed). - Exact-head
Clients / MobileandMobile Swift Domain / Mobile Swiftchecks were still in progress at verdict time. - Confidence gap only: no physical iOS/VoiceOver journey was run; source tracing and widget tests cover routing, not native announcement quality.
|
@builderbot review |
|
@buzz-security-review c99809a |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c99809a471
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Tom Brow <tomb@block.xyz>
|
@codex review |
|
@builderbot review |
|
@buzz-security-review bcfa8cf |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcfa8cf089
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Tom Brow <tomb@block.xyz>
Signed-off-by: Tom Brow <tomb@block.xyz>
|
@codex review |
|
@builderbot review |
|
@buzz-security-review 7ac39d1 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ac39d119c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Tom Brow <tomb@block.xyz>
|
@codex review |
|
@builderbot review |
|
@buzz-security-review 9928882 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99288821d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Tom Brow <tomb@block.xyz>
|
@codex review |
|
@builderbot review |
|
@buzz-security-review d3ee9ff |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3ee9ff17a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Tom Brow <tomb@block.xyz>
|
@codex review |
|
@builderbot review |
|
@buzz-security-review 9e7e41f |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Review clear
No actionable blockers found in this convergent source re-review of head 9e7e41f5e65147986533ee1b3d4fe958b314bb65 against base ef2aa1ae38fadcc0bc22b8bf6ed96b35933146be.
- The prior cold-page visibility finding is resolved: retained outer-root evidence is reconsidered after initial, older-page and deep-link rows become discoverable. The delayed-initial-page regressions cover both deleted-last-child and surviving-child outcomes.
- The fallback global-cache eviction repair preserves an affected root’s positive summary before dropping its last payload and uses the existing bounded recount path. The newest delta extends unknown-deletion uncertainty to valid broadcast
reply_count=1, descendant_count=0evidence. Its production-seam regressions cover empty ownership responses and exhausted retries without scanning every possible root. - The navigation contract remains intact in the reviewed paths: positive local/relay evidence, including direct-only broadcast counts, produces a reachable thread; established zero stays non-tappable; long-press Reply remains available. Conflicting unversioned live counts do not override a completed zero scan merely because they arrive later. The existing bounded reconciliation and conservative unknown-owner behavior are not being reopened.
Validation: source-only analysis on the pinned Blox host, including the new d3ee9ff17a..9e7e41f5e6 delta and its deletion-to-presentation consumers, integrated with preserved review evidence for 65cb69f9d7..d3ee9ff17a. Earlier backend/auth/protocol conclusions are carried forward, not independently re-certified. No checkout, build, test execution, PR-code execution or CI rerun was performed. The exact-head CI snapshot showed 25 successful, 21 skipped and 10 in-progress checks, including Clients / Mobile still running; this is not an all-green CI or runtime-validation claim.
Non-blocking coverage limit: the precise cold-branch race has a dedicated initial-page test, while older-page and deep-link recovery are source-verified through the shared reconciliation hook rather than equivalent dedicated race tests.
3 participants
Tapping a mobile channel message with no replies currently opens an empty thread. Only enable message-row thread navigation when the existing thread summary reports replies, including replies known to the relay but not loaded locally. Long-press → Reply still starts a new thread.
Added widget tests for zero replies, loaded replies, relay-only replies, and starting a thread through Reply. The zero-reply test failed before the fix and passes afterward.
Validation: all 227 channel-page tests, the full mobile pre-push checks, and all PR CI checks passed. The full repository gate failed in unchanged Rust ACP timing tests (idle reset, keepalive, and steering deadlines); this PR changes only mobile Dart files.
No duplicate open PR or issue found.