Skip to content

fix(mobile): fence queued relay publication by operation scope - #7536

Open
loganj wants to merge 1 commit into
fix/mobile-classification-consumer-5f397629from
fix/mobile-enqueue-fence-c3cdf65a
Open

loganj wants to merge 1 commit into
fix/mobile-classification-consumer-5f397629from
fix/mobile-enqueue-fence-c3cdf65a

Conversation

@loganj

@loganj loganj commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Summary (historical allocation; current owner split below)

Carry local operation currentness through relay backpressure to actual socket enqueue. A signed event waiting on the rate-limit gate must not be sent after cancellation merely because the connection is unchanged.

Uses a nested async operation scope (withRelayPublicationGuard) consumed synchronously by RelaySession after its final await; no asynchronous gap is introduced after the check, and existing publish/submit signatures remain compatible. Composer invitations and delivery retain draft/visit/upload/evidence guards; ChannelActions and SendMessage retain community guards. Accepted ACKs still count even if currentness changes after enqueue. No network-atomic permission guarantee is claimed.

Related issue (historical publication context)

The following related-issue and testing text records earlier candidates, including their then-open findings, sizes and private compositions; it is not current-head coverage or status. See the existing final-candidate section below for superseding pins and scope.
Transport finding from the fresh #7534 review. Serial base #7534 at 85ddfafe33b4e4fd2ee2f14c80918af25e0fb294; this is a genuine 174 additions +26 deletions =200 transport delta, not copied siblings. Observation completeness (policy/runtime/missing profile, F1) and public e9/#7391 composition remain open. No all-feedback closure.

Testing (historical candidate receipts)

At 156b7ec31b84ac642d4cc52e5d35debb88ea59a4, HEAD pinned in each final invocation:

  • just file-size-check: PASS.
  • just mobile-check: PASS (analyzer/format).
  • cd mobile && flutter test: 2151 passed, including the complete affected mobile relay/SDK code and production ChannelActions/SendMessage rate-gate regressions.
  • Four deterministic transport cases: kind9000/kind9 cancellation prevents outbound EVENT, healthy sends, cancellation after socketwrite preserves accepted ACK accounting. Real rate gate released deterministically; recording socket replaces network only.
  • Executed old-head source premise: kind9000 and kind9 tests both FAIL with unwanted outbound EVENT. Removing the new actual-enqueue check likewise makes both cancellation regressions FAIL; mutation restored before final checks.

No remote CI dispatch/poll/retry, native app or live-relay workflow execution; no whole-repository CI green claim. No visual-layout changes/screenshots.

Published SEND correction — 2026-09-10

Head 2b2a5c68111bacfd7de2486296fbc6a265ab0484; declared base 9bcf69fd428a988358302ea6af1a0328ecf000e9; actual own churn 584 including tests, strictly below600. Supersedes historical candidate pins and coverage below/above. Normal CI: https://github.com/block/buzz/actions/runs/34431159466 (completion pending).

Cumulative coverage remains conditional on the existing stack: #7527 owns scope/transport and capacity/generation foundation, #7534 classification, #7536 complete observed-evidence binding, and #7539 production-boundary journeys. This is not standalone authorization completeness at an intermediate parent or a backport to #7387. The original reviewed parents' gaps are acknowledged rather than relabeled as already fixed there.

R1/R2: empty latest required audience retires obsolete recipient proof only after successful operation-scope validation; retained recipients remain protected. Genuine community changes use a typed error and invitation StateError revalidates actual scope. Unavailable authority is not falsely described as a community switch. Accepted invitation prefixes remain irreversible and accounted for. A downstream onSend adapter can still encounter generic disconnected-session failure before the typed guard: no enqueue, but the specific community explanation is not exhaustive.

The 41-row matrix includes nine added real-boundary journeys. Production rows use the actual default provider/HTTP reader and real SendMessage → signed-event relay → session. Removing reader currentness or composer aggregation independently leaks kind9; invitation counterparts leak accepted kind9000. Removing only composer guardedDelivery leaks kind9 after real draft editing while SendMessage's wrapper remains. Removing default-provider profile forwarding fails consent/post-ACK continuity (first failing assertion is unwanted kind9, not an independently logged kind9000 failure). Pure capacity and no-capacity profile-revocation rows isolate those causes; the waiting-revocation row does not independently separate coordinates from aged capacity. EMPTY/TYPED mutations fail their observable delivery/UI assertions. Source was restored. These are local mutation receipts on equivalent production, not new published-head mutation executions.

Validation: exact reviewed candidate full-mobile suites passed 2137/2152/2162/2188 for #7527/#7534/#7536/#7539. All mobile production AND test tree objects are byte-identical after mechanical Desktop-only carry; no unnecessary mobile full rerun is claimed. Own-range source-size gates pass after carry; format/analyzer receipts apply to unchanged mobile bytes. Desktop JS package 6450/6450 passed; forum 19+63 and video 7 E2E / 4 buffering unit cases are scoped receipts on identical runtime inputs, not complete Playwright/CI passes.

Private16 is now 79d3a431d08413225dda88f7fea7ca48d2a69e25: same mobile tree as reviewed 0a28720e89b5319ebc682ea98ef73fae53f5e0d0 (full2231); forum/video test repairs carried once each. Production correspondence to public SEND was independently byte-verified. Earlier same-production restart11 (ten classification plus original exact-draft) and mounted provenance1 receipts are reused, not new final-head executions. Public versus former 668f1ffb and private versus cc6c3210 mobile growth is +19/-6 production and +343/-200 tests; allocation is not a net saving. This is 16 constituents, not all20, native/live-relay/VoiceOver or release acceptance. #7391's independent critique is not closed by composition.

Independent delta review e6ec accepted the exact mobile candidates with the qualifications above; old GitHub approvals are not transferred. Normal push-triggered CI is running, not yet green; security skips are not a security verdict. No rerun wave, dispatch, merge or new PR. Evidence: WORK_LOGS/MOBILE_FEEDBACK_CLASSIFICATION_20260909/GREEN_SEND_DELTA_REVIEW_D231.md, GREEN_SEND_IMPLEMENTATION.md, and GREEN_STACK_INTEGRATION.md / GREEN_6A5/.

Mechanical docs-carry note — 2026-09-10

Head mechanically carried to 4660aae48fc66f2547f835557032df641cad6f45 (corrected parent #7534 a3be0741b… + this PR's original own commits, zero conflicts, original messages/authors/DCO preserved). The carry is docs-only — the #7531 review-5162334206 /// correction on mobile/lib/shared/mentions/selected_mention_authorization.dart (comment-stripped file equality; mobile/test bytes identical to 2b2a5c68111bacfd7de2486296fbc6a265ab0484); earlier "Head 2b2a5c68111bacfd7de2486296fbc6a265ab0484"/current-HEAD mappings above are dated receipts for their pre-carry heads, including their CI links. Own churn is unchanged at 584 including tests (inherited docs are not counted as own); no executable delta. Normal CI on the carried head: https://github.com/block/buzz/actions/runs/34436597609.

Workflows helper test carry — 2026-09-10

Head rebased to 40573cfac0391974400a9f65a1c9713471f35e9e (single carry commit, author and message preserved, on corrected parent #7534 42e830f65596198404f529b57bdb52d0998ce3b0); no conflicts, no merge commits. Own range vs the corrected parent is unchanged at 480 additions + 104 deletions = 584 lines including tests; the only old-head→new-head content delta is the carried #7531 Desktop E2E test repair in desktop/tests/e2e/workflows.spec.ts (+15/−3) and desktop/tests/e2e/workflow-local-controls.spec.ts (+7/−2) — mobile production, mobile tests, and API docs are byte-identical old→new. Current-head review 5162776802 predates this head movement. Normal CI on the new head is running; no green claim is made here.

Workflow regression test carry — 2026-09-10

Head rebased to 06a016e1cb69e9fbdcda9826a762c08ea800cba9 (single carry commit, author and message preserved, on parent #7534 a22b79fa8c6c2db81f37953e9c0f7b168b1043d6); no conflicts, no merge commits. Own range vs the new parent is unchanged at 480 additions + 104 deletions = 584 lines including tests; the only old-head→new-head content delta is the carried #7531 test-only +24/−0 regression coverage in desktop/tests/e2e/workflows.spec.ts — mobile production, mobile tests, and API docs are byte-identical old→new. The open review state on this PR (change request 5163167716 and the dismissed 5163168834) predates this head movement and is not resolved by this test-only carry. Normal CI on the new head: https://github.com/block/buzz/actions/runs/34444700331 (no green claim is made here).

@loganj
loganj requested a review from a team as a code owner September 9, 2026 18:39

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — REQUEST CHANGES on exact head 156b7ec31b84ac642d4cc52e5d35debb88ea59a4 (base 85ddfafe33b4e4fd2ee2f14c80918af25e0fb294).

[P2] Bind the regression to the production operation guards

mobile/test/shared/relay/relay_enqueue_fence_test.dart:41-57 manually wraps both branches in withRelayPublicationGuard(ensureCurrent, ...). That test-authored outer guard independently reaches RelaySession.publish, so the test proves the generic enqueue fence but not either production propagation seam it is intended to protect.

Mutation evidence at this exact head:

  • Bypassing the ChannelActions.addMembers guard at mobile/lib/features/channels/channel_management_actions.dart:107-118 left all 4 new tests passing.
  • Restoring that site and bypassing the SendMessage.call guard at mobile/lib/features/channels/send_message_provider.dart:100-112 also left all 4 passing.
  • As a positive control, removing the actual enqueue check at mobile/lib/shared/relay/relay_session.dart:364 made both cancellation rows fail with an unwanted EVENT.

The suite therefore permits either production guard to be deleted or miswired while remaining green, leaving stale invitations or messages able to publish after the relevant operation becomes invalid. This misses the repository requirement for regression coverage through the production seam (TESTING.md:25-31).

Author action: add deterministic rate-gate rows that invalidate through each production object's own isCommunityValid / isDeliveryValid callback, without an enclosing test guard. Mutation of each production wrapper independently should make its corresponding behavior test fail. Keep a separate direct transport test with the outer guard if useful. Include user-facing coverage for the distinct compose/delivery predicates where they own cancellation, draft preservation, snackbar classification, or partial-invitation state.

Verification owner: author supplies the causal production-seam tests; reviewer reruns independent mutations of each wrapper.

Other evidence

No separate production-code defect was established in the reviewed systems or user-trust paths. Nested/concurrent Zone scopes remain isolated; the post-rate-gate generation/socket and operation checks run without an intervening await before pending registration/socket send; reconnect/dispose reject old-generation work; guard failure creates no ACK waiter; accepted events retain normal OK accounting; and existing publish/submit signatures remain unchanged.

  • Full cd mobile && flutter test: 2151 passed at exact head, clean tree.
  • Focused enqueue-fence regression: 4/4 passed.
  • git diff --check 85ddfafe33b4e4fd2ee2f14c80918af25e0fb294...156b7ec31b84ac642d4cc52e5d35debb88ea59a4: passed.
  • Mobile CI, DCO, Semgrep, zizmor, and reported integration checks inspected as passing; unrelated Desktop/macOS checks were still running during review.

Confidence gaps, not additional author defects: no native/live-relay mobile journey was run; unrelated in-progress CI remains owned by its named gate.

@loganj

loganj commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Updated response to review 5159612790 and R1/R2; this replaces the prior candidate evidence in place.

Published SEND correction — 2026-09-10

Head 2b2a5c68111bacfd7de2486296fbc6a265ab0484; declared base 9bcf69fd428a988358302ea6af1a0328ecf000e9; actual own churn 584 including tests, strictly below600. Supersedes historical candidate pins and coverage below/above. Normal CI: https://github.com/block/buzz/actions/runs/34431159466 (completion pending).

Cumulative coverage remains conditional on the existing stack: #7527 owns scope/transport and capacity/generation foundation, #7534 classification, #7536 complete observed-evidence binding, and #7539 production-boundary journeys. This is not standalone authorization completeness at an intermediate parent or a backport to #7387. The original reviewed parents' gaps are acknowledged rather than relabeled as already fixed there.

R1/R2: empty latest required audience retires obsolete recipient proof only after successful operation-scope validation; retained recipients remain protected. Genuine community changes use a typed error and invitation StateError revalidates actual scope. Unavailable authority is not falsely described as a community switch. Accepted invitation prefixes remain irreversible and accounted for. A downstream onSend adapter can still encounter generic disconnected-session failure before the typed guard: no enqueue, but the specific community explanation is not exhaustive.

The 41-row matrix includes nine added real-boundary journeys. Production rows use the actual default provider/HTTP reader and real SendMessage → signed-event relay → session. Removing reader currentness or composer aggregation independently leaks kind9; invitation counterparts leak accepted kind9000. Removing only composer guardedDelivery leaks kind9 after real draft editing while SendMessage's wrapper remains. Removing default-provider profile forwarding fails consent/post-ACK continuity (first failing assertion is unwanted kind9, not an independently logged kind9000 failure). Pure capacity and no-capacity profile-revocation rows isolate those causes; the waiting-revocation row does not independently separate coordinates from aged capacity. EMPTY/TYPED mutations fail their observable delivery/UI assertions. Source was restored. These are local mutation receipts on equivalent production, not new published-head mutation executions.

Validation: exact reviewed candidate full-mobile suites passed 2137/2152/2162/2188 for #7527/#7534/#7536/#7539. All mobile production AND test tree objects are byte-identical after mechanical Desktop-only carry; no unnecessary mobile full rerun is claimed. Own-range source-size gates pass after carry; format/analyzer receipts apply to unchanged mobile bytes. Desktop JS package 6450/6450 passed; forum 19+63 and video 7 E2E / 4 buffering unit cases are scoped receipts on identical runtime inputs, not complete Playwright/CI passes.

Private16 is now 79d3a431d08413225dda88f7fea7ca48d2a69e25: same mobile tree as reviewed 0a28720e89b5319ebc682ea98ef73fae53f5e0d0 (full2231); forum/video test repairs carried once each. Production correspondence to public SEND was independently byte-verified. Earlier same-production restart11 (ten classification plus original exact-draft) and mounted provenance1 receipts are reused, not new final-head executions. Public versus former 668f1ffb and private versus cc6c3210 mobile growth is +19/-6 production and +343/-200 tests; allocation is not a net saving. This is 16 constituents, not all20, native/live-relay/VoiceOver or release acceptance. #7391's independent critique is not closed by composition.

Independent delta review e6ec accepted the exact mobile candidates with the qualifications above; old GitHub approvals are not transferred. Normal push-triggered CI is running, not yet green; security skips are not a security verdict. No rerun wave, dispatch, merge or new PR. Evidence: WORK_LOGS/MOBILE_FEEDBACK_CLASSIFICATION_20260909/GREEN_SEND_DELTA_REVIEW_D231.md, GREEN_SEND_IMPLEMENTATION.md, and GREEN_STACK_INTEGRATION.md / GREEN_6A5/.


Addendum — response to review 5162293777 at 2b2a5c68111bacfd7de2486296fbc6a265ab0484 (2026-09-10)

The finding is confirmed: at this head the production wrapper withRelayPublicationGuard(ensureAuthorizationCurrent, ...) exists at compose_bar_widget.dart:532-539 (passed into delivery at :651), but the 156 ComposeBar tests and the 171-combined relevant suite do not falsify its deletion, and relay_enqueue_fence_test.dart proves the generic transport fence with a test-authored outer guard rather than that this composer installs it. No new production-code defect is established at this head; the gap is exactly the missing production-seam falsification.

The requested production-seam falsification lives at #7539 722d1aa4e3411b02894aac187a02d40140d8a482: its classification tests (:145-159, :255-276, :372-401) hold a real SendMessage/session at the real relay capacity gate, edit the controller mid-wait, then release the gate and assert no kind 9 EVENT reaches the socket while the intended draft/error state is preserved. The GUARDED_DELIVERY mutant removes only the composer wrapper and leaks kind9 while SendMessage's own wrapper remains; that removal fails the production-path assertions.

Attribution qualifications retained: those mutation receipts were executed on the reviewed candidate trees — full suites 2137 (#7527) / 2152 (#7534) / 2162 (#7536) / 2188 (#7539) and private 2231, carried through the verified byte-identical mobile production and test trees after the Desktop-only changes — not freshly rerun at this public SHA; verified tree identity is the carry basis, not same-SHA execution. #7539 also carries a real on-head approval (5162354965 at 722d1aa), but its same-head CI has a Desktop-lane failure under separate active diagnosis; no green/landed status is claimed and no approval is transferred.

Please assess this as conditional descendant proof of the production seam — not as 156 own-head tests now protecting it, and not as closure of this review. If this merge unit must carry the falsification standalone, that is an explicit owner policy decision we are asking to be made before bytes are moved or duplicated; no backport, restack, merge, re-approval, or review closure is asserted here.

2026-09-10 mechanical-carry note (post-response): the branch heads referenced above as current (e.g. 2b2a5c68111bacfd7de2486296fbc6a265ab0484, and descendant stack pins) have since been mechanically docs-carried — this PR now at 4660aae48fc66f2547f835557032df641cad6f45, with the same own commits — for the #7531 review-5162334206 docs-only correction (comment-stripped equality, mobile tests byte-identical, no executable delta; current pins and normal-CI links in each PR body's dated carry note). The exact-head evidence and mutant/attribution pins above remain valid dated receipts for their pre-carry heads; no approval or coverage claim is changed by this note.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Reviewed: c78d8f4accb8bbb3610be149fd0733f04383da91..883a16baeebf07044bf96addba7f095a22dfaf0d (exact live head rechecked)

Risk: high — this changes message/invitation publication across relay backpressure, including cancellation, evidence currentness, and ACK settlement.

Blocking findings

  1. Production selected-evidence currentness is disconnected from the enqueue fence.

    mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:494-497,525-539 now relies on each SelectedMentionAuthorization.isCurrent callback for invitation and message publication. But the production reader constructs every result at mobile/lib/shared/mentions/selected_mention_authorization.dart:178-182 without isCurrent, selecting the always-true default at lines 3,25-30. The apparent production capability, RelaySessionNotifier.retainPublicationEvidence() (mobile/lib/shared/relay/relay_session.dart:415-424), has no caller under mobile/lib. This head also removes the prior concrete userCacheProvider.profileEventOrder comparison.

    The new capacity test injects its own meaningful callback at mobile/test/features/channels/compose_bar_test/classification_tests.dart:24-40; it therefore proves that a test-authored capability is consulted, not that the real reader creates one. In production, newer profile/ownership/policy evidence arriving while an invitation or message waits at relay capacity cannot expire the queued publication.

    Author action: populate real selected-evidence results from production-owned authoritative-read/currentness evidence (including replacement and session/rate-generation retirement), or restore an equivalent production-owned revision comparison. Drive the capacity-wait regression through the default production reader and real evidence replacement; bypassing the production binding must cause the signed kind 9000/9 leak assertion to fail.

    Verification owner: author supplies the production binding and causal regression; reviewer re-runs production-callsite mutation and capacity-wait behavior.

  2. The final composer message fence is not protected by a causal production regression.

    guardedDelivery at mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:532-539 is the distinct outer composer-operation fence for final message enqueue. Replacing it with direct onSend(...) left the complete compose-bar test file green (157/157). The new real-capacity widget row queues an invitation (kind 9000), not the final message (kind 9), so it cannot detect removal of this fence.

    Author action: add a production-driven composer case that reaches a real SendMessage/session rate gate, invalidates the operation through edit, revisit, upload generation, or community switch, releases the gate, and asserts no kind 9 EVENT plus the correct newer-draft/error state. Mutating only guardedDelivery must fail behaviorally.

    Verification owner: author supplies the causal regression; reviewer mutation-verifies it.

What is fixed and what was traced

The prior production-seam blocker is resolved: the replacement tests directly enter ChannelActions.isCommunityValid and SendMessage.isDeliveryValid. Independently removing either production wrapper makes its cancellation row fail with an unwanted signed kind 9000 or kind 9 EVENT. Nested guards compose, concurrent Zone scopes remain isolated, reconnect/dispose generation rejects old gated work, and there is no await between the final connection/operation checks and socket enqueue. Guard failure occurs before ACK timer/pending registration; once transmitted, accepted ACKs settle normally. Existing publish/submit signatures remain compatible.

Validation at exact clean head 883a16baeebf07044bf96addba7f095a22dfaf0d

  • cd mobile && flutter test: 2153 passed.
  • Focused relay enqueue + compose-bar tests: 162 passed.
  • Independent production-wrapper mutations: ChannelActions bypass failed with unwanted kind 9000; SendMessage bypass failed with unwanted kind 9; restored suite passed 5/5.
  • Final-composer-fence mutation: full compose-bar file remained green 157/157, establishing finding 2.
  • just mobile-check: pass (560 files formatted unchanged; analyzer clean).
  • CHECK_FILE_SIZES_BASE=c78d8f4accb8bbb3610be149fd0733f04383da91 just file-size-check: pass.
  • git diff --check: pass.
  • Same-shell git rev-parse HEAD matched and the worktree was clean for final reviewer runs.

Manual/native evidence: not run; no visual-layout change. No live-relay/native iOS workflow was independently witnessed.

Residual risk: CI still had named jobs in progress at the last freshness check; no red required gate was observed. Native/live-relay behavior remains a reviewer/tooling confidence gap, separate from the concrete defects above.

@loganj
loganj force-pushed the fix/mobile-classification-consumer-5f397629 branch from c78d8f4 to 9bcf69f Compare September 10, 2026 02:52
@loganj
loganj force-pushed the fix/mobile-enqueue-fence-c3cdf65a branch from 883a16b to 2b2a5c6 Compare September 10, 2026 02:52

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — REQUEST CHANGES on exact head 2b2a5c68111bacfd7de2486296fbc6a265ab0484 against base 9bcf69fd428a988358302ea6af1a0328ecf000e9.

Risk: high — this changes message/invitation authorization currentness across asynchronous upload and relay backpressure, immediately before signed-event enqueue.

[P1] Bind the production ComposeBar publication fence to a falsifiable regression

The final composer operation fence is installed at mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:532-539 and passed into delivery at line 651. Independently replacing that withRelayPublicationGuard(ensureAuthorizationCurrent, ...) wrapper with direct onSend(...) left all 156 ComposeBar tests green and also left the combined relevant ComposeBar, selected-observation, and relay enqueue-fence suite green (171/171).

mobile/test/shared/relay/relay_enqueue_fence_test.dart proves the generic transport fence and nested-scope behavior using a test-authored outer guard. It does not prove that ComposeBar installs its production guard. The suite therefore permits this production seam to be deleted while remaining green. A future refactor could then publish a kind 9 EVENT after the originating compose operation becomes stale while waiting at the relay rate-limit gate.

This violates the repository’s explicit falsifiability requirement: “a guard whose removal doesn't fail any test protects nothing” (AGENTS.md:188-192; TESTING.md:25-31). It also leaves the PR’s central user-trust invariant unprotected at its real production boundary.

Author action: add a deterministic regression through the real ComposeBar → SendMessage → relay-session path that queues the message EVENT behind the real rate-limit gate, invalidates the originating compose operation (edit/revisit/unmount/new-send, according to the intended lifecycle), releases the gate, and asserts that no EVENT reaches the socket while preserving the intended draft/error state. Mutation of only the ComposeBar wrapper at line 536 must make that test fail; restoring it must pass.

Verification owner: author supplies the production-path regression; reviewer repeats the exact red-without/green-with mutation at the pinned head.

Integrated trace and validation

No separate production-code defect was established. The operation fence otherwise composes through ComposeBar and provider scopes and executes in RelaySessionNotifier.publish after rate-limit waiting and immediately before pending registration/socket enqueue. Session-local coordinate evidence is retired on newer signed socket/HTTP observations, rebuild/dispose, socket supersession, and capacity-epoch changes. The selected-authorization reader fails closed for unavailable/ambiguous membership and missing owner policy. Upload/send cancellation retains or restores the originating draft and attachments without overwriting newer edits.

At exact clean head:

  • Full cd mobile && flutter test: 2,162 passed in the systems/integration lane. A separate full-suite run encountered one activity_provider_test.dart failure that passed in isolation; treated as an unrelated flake, not author action.
  • Focused observation/enqueue/authorization tests: 24 passed.
  • ComposeBar: 156 passed.
  • Send-message + channel-management-provider + relay-session: 76 passed.
  • flutter analyze: pass, no issues.
  • git diff --check: pass.
  • Production-fence mutation: incorrectly green (156/156 ComposeBar; 171/171 combined relevant suite), establishing the blocking test defect.
  • Live GitHub head remained 2b2a5c68111bacfd7de2486296fbc6a265ab0484; all reported checks are now terminal green or path-filtered skipped.

Manual/native evidence: no real-device or live-relay race was run. Unit/widget seams establish local observed-currentness, not network-atomic authorization against unseen remote changes; this is a confidence gap, not additional author action.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent — REQUEST CHANGES on exact head 4660aae48fc66f2547f835557032df641cad6f45 against base a3be0741b301a47d5affbc89105b57cfa381d418.

Risk: high — this change carries selected relay-evidence and compose-operation currentness across asynchronous preparation and relay backpressure to signed-event enqueue.

[P1] Bind the final production ComposeBar message fence to a causal regression

The final operation fence is installed by guardedDelivery at mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:532-539, wrapping the real onSend call so draft/evidence currentness is checked after relay backpressure and immediately before enqueue.

An independent mutation replacing only that wrapper with direct onSend(...) left the complete owning test file green: 156/156 ComposeBar tests passed. The new queued cases in mobile/test/shared/mentions/selected_observation_test.dart:186-208 manually install withRelayPublicationGuard around a directly constructed SignedEventRelay.submit(kind: 9000, ...). They prove the production evidence capability and generic transport fence, but bypass ComposeBar’s guardedDelivery seam and exercise an invitation event rather than the final kind 9 message. Consequently, deletion of the load-bearing composer-to-transport binding remains invisible to its owning suite.

That violates the repository’s explicit falsifiability requirement: “a guard whose removal doesn't fail any test protects nothing” (AGENTS.md:188-192; TESTING.md:25-31). It leaves this PR’s final-message stale-operation invariant unprotected against future wiring regressions.

Author action: add a deterministic production-path regression through ComposeBar → SendMessage → SignedEventRelay → RelaySession. Queue the final kind 9 message behind the real rate-limit gate, invalidate the originating draft/visit/evidence while it waits, release the gate, and assert no socket EVENT plus the intended draft/error state. Removing only guardedDelivery must make that regression fail behaviorally; restoring it must pass.

Verification owner: author supplies the production-boundary regression and mutation receipt; reviewer independently repeats the single-wrapper red/green mutation at the replacement head.

Integrated review and exact-head evidence

No separate production-code defect was established. The former disconnected evidence capability is now substantively wired: the production reader retains session/rate-generation scope, snapshots exact roster/runtime/profile/policy coordinates, and returns the composed isCurrent callback. RelayEvidenceClock is bounded, retires evicted capabilities, observes only signature-verified relevant kinds, isolates coordinates, and clears on rebuild, socket supersession, and disposal. The composed guard runs after rate-gate waiting and before ACK registration/socket send; accepted writes continue normal settlement. Draft restoration, community-change reporting, and accepted invitation-prefix disclosure remain coherent.

At exact clean HEAD 4660aae48fc66f2547f835557032df641cad6f45:

  • Full cd mobile && flutter test: 2,162 passed.
  • selected_observation_test.dart: 10 passed.
  • Full compose_bar_test.dart: 156 passed.
  • dart format --output=none --set-exit-if-changed lib test: pass, 562 files unchanged.
  • flutter analyze: pass, no issues.
  • Explicit-base CHECK_FILE_SIZES_BASE=a3be074… just file-size-check: pass.
  • git diff --check: pass.
  • Live GitHub base/head remained exact immediately before submission; mobile, Mobile Swift, relay/integration, DCO, Semgrep, zizmor, Windows, macOS, and two desktop smoke shards were green. Desktop Core and smoke shards 2/4 remained in progress; no current red required gate was observed.

Confidence gaps: no native-device/live-relay race was witnessed. Local tests establish observed-currentness, not network-atomic authorization against unseen remote events. Pending unrelated desktop CI owns its own completion. These are reviewer/CI-owned gaps, not additional author work.

@loganj
loganj force-pushed the fix/mobile-enqueue-fence-c3cdf65a branch from 4660aae to 40573cf Compare September 10, 2026 05:30
@loganj
loganj force-pushed the fix/mobile-classification-consumer-5f397629 branch from a3be074 to 42e830f Compare September 10, 2026 05:30

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Re-reviewed exact head 40573cfac0391974400a9f65a1c9713471f35e9e against base 42e830f65596198404f529b57bdb52d0998ce3b0.

[P1] Bind the final ComposeBar publication fence to a causal production-path regression

The load-bearing final-message wrapper remains guardedDelivery at mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:532-539, used for text delivery at line 651 and attachment delivery at line 676. However, the new queued rows in mobile/test/shared/mentions/selected_observation_test.dart:186-208 manually wrap a directly constructed SignedEventRelay.submit(kind: 9000, ...). They test a generic invitation-event fence, bypass ComposeBar.guardedDelivery, and never queue the final kind-9 message.

I replaced only ComposeBar's withRelayPublicationGuard(ensureAuthorizationCurrent, ...) wrapper with direct onSend(...) and ran the complete owning suite. All 156/156 tests still passed. I then restored the file and confirmed a clean tree at the exact head. This mutation proves the production binding can be removed or miswired without the owning regression suite noticing, leaving stale kind-9 publication after edit/revisit/upload-generation/evidence invalidation undetected. That fails the repository's falsifiability requirement in TESTING.md:25-31 (and AGENTS.md:188-192).

Author action: add a deterministic regression through the real production chain ComposeBar → SendMessage → SignedEventRelay → RelaySession: hold the final kind-9 event at the real RelayRateLimitGate, invalidate the originating compose operation/evidence, release the gate, then assert that no socket EVENT is emitted and that draft/error recovery is correct. Removing only guardedDelivery must make that test fail behaviorally; restoring it must pass. Evidence in a descendant PR does not cover this merge unit.

Verification owner: author supplies the on-head production-path regression; reviewer independently repeats the single-wrapper red/green mutation.

No separate production-code defect was established. The evidence-currentness capability appears coherently wired through session/rate epoch and roster/runtime/profile/policy coordinates; observation updates, eviction/session replacement/disposal, and the final pre-enqueue guard fail closed in the reviewed paths.

Exact-head evidence

  • PASS — full mobile suite: 2162 passed in one independent run. A second run reported 2161 passed / 1 failed from a temp-directory double-delete in voice_note_recording_test.dart:730; that exact test passed alone, so this is presently a confidence gap rather than author action.
  • PASS — selected_observation_test.dart: 10 passed.
  • PASS — selected_observation_test.dart + relay_enqueue_fence_test.dart: 15 passed.
  • PASS — pristine full compose_bar_test.dart: 156 passed.
  • Defect reproduced — single guardedDelivery bypass mutation: full compose_bar_test.dart incorrectly remained 156/156 green in three independent runs; trees restored cleanly.
  • PASS — just mobile-check / formatting and analyzer clean.
  • PASS — base-aware just file-size-check.
  • PASS — git diff --check.
  • Freshness immediately before review: live PR remained OPEN at the exact base/head; local HEAD matched and tree was clean.

Confidence gaps — no author action currently

  • Native iOS/live-relay timing was not independently exercised. Verification owner: integration/release validation.
  • Current-head CI still had seven jobs in progress. Cancelled/failing display rows inspected belonged to the superseded run, not a demonstrated PR-caused required-gate failure. Verification owner: required CI gates; reclassify only if the current run fails causally.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:bot: Jude’s code review agent

Re-reviewed exact head 40573cfac0391974400a9f65a1c9713471f35e9e against base 42e830f65596198404f529b57bdb52d0998ce3b0.

[P1] Bind the final ComposeBar publication fence to a causal production-path regression

The load-bearing final-message wrapper remains guardedDelivery at mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:532-539, used for text delivery at line 651 and attachment delivery at line 676. However, the new queued rows in mobile/test/shared/mentions/selected_observation_test.dart:186-208 manually wrap a directly constructed SignedEventRelay.submit(kind: 9000, ...). They test a generic invitation-event fence, bypass ComposeBar.guardedDelivery, and never queue the final kind-9 message.

I replaced only ComposeBar's withRelayPublicationGuard(ensureAuthorizationCurrent, ...) wrapper with direct onSend(...) and ran the complete owning suite. All 156/156 tests still passed. I then restored the file and confirmed a clean tree at the exact head. This mutation proves the production binding can be removed or miswired without the owning regression suite noticing, leaving stale kind-9 publication after edit/revisit/upload-generation/evidence invalidation undetected. That fails the repository's falsifiability requirement in TESTING.md:25-31 (and AGENTS.md:188-192).

Author action: add a deterministic regression through the real production chain ComposeBar → SendMessage → SignedEventRelay → RelaySession: hold the final kind-9 event at the real RelayRateLimitGate, invalidate the originating compose operation/evidence, release the gate, then assert that no socket EVENT is emitted and that draft/error recovery is correct. Removing only guardedDelivery must make that test fail behaviorally; restoring it must pass. Evidence in a descendant PR does not cover this merge unit.

Verification owner: author supplies the on-head production-path regression; reviewer independently repeats the single-wrapper red/green mutation.

No separate production-code defect was established. The evidence-currentness capability appears coherently wired through session/rate epoch and roster/runtime/profile/policy coordinates; observation updates, eviction/session replacement/disposal, and the final pre-enqueue guard fail closed in the reviewed paths.

Exact-head evidence

  • PASS — full mobile suite: 2162 passed in one independent run. A second run reported 2161 passed / 1 failed from a temp-directory double-delete in voice_note_recording_test.dart:730; that exact test passed alone, so this is presently a confidence gap rather than author action.
  • PASS — selected_observation_test.dart: 10 passed.
  • PASS — selected_observation_test.dart + relay_enqueue_fence_test.dart: 15 passed.
  • PASS — pristine full compose_bar_test.dart: 156 passed.
  • Defect reproduced — single guardedDelivery bypass mutation: full compose_bar_test.dart incorrectly remained 156/156 green in three independent runs; trees restored cleanly.
  • PASS — just mobile-check / formatting and analyzer clean.
  • PASS — base-aware just file-size-check.
  • PASS — git diff --check.
  • Freshness immediately before review: live PR remained OPEN at the exact base/head; local HEAD matched and tree was clean.

Confidence gaps — no author action currently

  • Native iOS/live-relay timing was not independently exercised. Verification owner: integration/release validation.
  • Current-head CI still had seven jobs in progress. Cancelled/failing display rows inspected belonged to the superseded run, not a demonstrated PR-caused required-gate failure. Verification owner: required CI gates; reclassify only if the current run fails causally.

@jedwards27
jedwards27 dismissed their stale review September 10, 2026 05:43

Duplicate submission caused by a transient GraphQL error after the first exact-head review had already been accepted.

Signed-off-by: Logan Johnson <loganj@squareup.com>
@loganj
loganj force-pushed the fix/mobile-classification-consumer-5f397629 branch from 42e830f to a22b79f Compare September 10, 2026 06:19
@loganj
loganj force-pushed the fix/mobile-enqueue-fence-c3cdf65a branch from 40573cf to 06a016e Compare September 10, 2026 06:19

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing exact head 06a016e1cb69e9fbdcda9826a762c08ea800cba9 against base a22b79fa8c6c2db81f37953e9c0f7b168b1043d6.

[P1] Add a causal production-path regression for the final ComposeBar publication fence

The load-bearing operation wrapper is installed at mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:532-539 and used by text and attachment delivery at lines 651 and 676. The new queued tests at mobile/test/shared/mentions/selected_observation_test.dart:186-208 instead manually guard a direct SignedEventRelay.submit(kind: 9000); they neither traverse ComposeBar.guardedDelivery nor queue the final kind-9 message.

This leaves the PR's central safety seam unfalsified. Replacing only the ComposeBar wrapper with direct onSend(...) leaves the complete owning ComposeBar suite green (156/156) and also leaves the combined ComposeBar, selected-observation, and relay-enqueue suites green (171/171). A future removal or miswiring of this wrapper could therefore allow a stale final message to reach the socket after a newer edit, revisit/unmount, upload change, or evidence expiry while every relevant on-head test still certifies the behavior. That fails the production-seam/falsifiability requirement in TESTING.md:25-31 and AGENTS.md:188-192.

Author action: add an on-head deterministic regression through the real ComposeBar → SendMessage → SignedEventRelay → RelaySession chain. Hold the final kind-9 event at the real RelayRateLimitGate, invalidate the originating compose operation (a newer edit is the strongest case), release the gate, and assert that no socket EVENT is emitted and the newer draft/error state is not overwritten. Removing only guardedDelivery must make the test fail behaviorally; restoring it must pass.

Verification owner: author supplies the causal on-head regression; reviewer repeats the single-wrapper red/green mutation.

Evidence at this head

  • cd mobile && flutter test: 2162/2162 passed.
  • just mobile-check: 562 files format-clean; analyzer reported no issues.
  • Explicit-base CHECK_FILE_SIZES_BASE=a22b79f… just file-size-check: 10/10 policy tests passed.
  • git diff --check: passed.
  • Current-head Mobile, Mobile Swift, DCO, Semgrep, zizmor, relay/integration results, macOS build, and Windows build were green or correctly skipped. Four unrelated Desktop jobs remained in progress at final check; superseded-run cancellations are not attributed to this head.

No separate production-code defect was established. Native iOS/live-relay timing remains a reviewer/release confidence gap, not additional author action. The documented residual race against an unseen remote authorization change is also not a new blocker.

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