Skip to content

fix(mobile): disclose irreversible invitation completion - #7527

Open
loganj wants to merge 1 commit into
fix/mobile-invitations-20260905from
fix/mobile-invitation-commitment-20260909
Open

fix(mobile): disclose irreversible invitation completion#7527
loganj wants to merge 1 commit into
fix/mobile-invitations-20260905from
fix/mobile-invitation-commitment-20260909

Conversation

@loganj

@loganj loganj commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Inviting mentioned people or agents changes channel membership even if the message later stops or fails. Make that commitment explicit before Invite, relabel the media action to Stop remaining when invitations start, and report how many acknowledged invitations remain in effect when delivery does not finish. Direct the sender to review members in the original community/channel and check the remaining draft before retrying; attachments may need reattaching after leaving.

This does not claim a multi-event transaction or remove independently valid members. Stopping can prevent remaining work, not undo an invitation already accepted (including an acknowledgement that arrives after Stop).

Related issue (historical allocation)

Dependent followup to #7390 and its requested-change review #7390 (review) . Closest existing work is #7390; this delta requires its explicit invitation and source-lifecycle fencing. Base remains fix/mobile-invitations-20260905, now rebased onto #7531. Historical own ranges: #7390 466 lines; this followup 115 lines. All original invitation/lifecycle tests are retained. #7394 is included by the chosen serial integration sequence, not an inherent dependency of the historical invitation feature. Fresh persisted agent classification (#7387) is separate and not claimed fixed here.

Testing (historical candidate)

  • At historical 97147e9a9f0aae7421067ca121d34701cda7d224: just mobile-check passes and full flutter test 2,132 passed. Historical 2,085-test evidence applies only to old a6b55f919.
  • Production-composer regressions cover accepted first invitation followed by Stop, media source revisit, later invitation refusal, or scope switch immediately after successful relay OK. Assert no message, text retention when still available, actual accepted-prefix event count, disclosure, Stop remaining label, original destination and truthful recovery wording. The media revisit test verifies its selected attachment is absent on return; no complete durable-media retention is promised. Moving acceptance accounting below ChannelActions’ scope fence makes the new scope-switch test fail.
  • No device/simulator journey or screenshots captured; widget coverage only. Repository-wide just ci not run. Independent review and exact-head CI remain gates; no merge requested.

Historical serial integration carry (2026-09-09)

Historical HEAD 97147e9a9f0aae7421067ca121d34701cda7d224; own PR range 115 changed lines, including tests. Existing PR retained; no duplicate feature patch or merge. Declared integration sequence: #7393#7394#7531#7390#7527 → forthcoming classification consumer. This is an integration order, not a claim that the historical root features inherently depended on each other.

Final carried HEAD: just mobile-check passes; full flutter test: 2132 passed. These supersede prior standalone counts for this head. No new screenshot or composed runtime capture claimed. Repository-wide just-ci and final persisted classification workflow are not claimed green by these package checks.

Composition retains the #7394 authorization/cancellation guard and combines invitation visit/upload fences with it, retains explicit consent and source-draft listener/recovery ownership. #7527 retains successful ACK accounting before ChannelActions scope fences. Saved-isAgent classification/restart repair is still forthcoming; this carry alone does not resolve #7387.

Published SEND correction — 2026-09-10

Head 2943008ec56589140cccd37f507a1393e64a4671; declared base 069c8b71a4b43edc3dae946e14d2523fc8a16d29; actual own churn 540 including tests, strictly below600. Supersedes historical candidate pins and coverage below/above. Normal CI: https://github.com/block/buzz/actions/runs/34431155737 (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 84f61093be51f2824debdf4b65e76d12652acfbf (corrected parent #7390 d080eccb1… + 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 2943008ec56589140cccd37f507a1393e64a4671); earlier "Head 2943008ec56589140cccd37f507a1393e64a4671"/current-HEAD mappings above are dated receipts for their pre-carry heads, including their CI links. Own churn is unchanged at 540 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/34436598895.

Workflows helper test carry — 2026-09-10

Head rebased to f098f9aa877235320dade5a76ae1b2ac02e4fe70 (single carry commit, author and message preserved, on corrected parent #7390 5f8744a3fb571b6dd9a6d3853ae254694ab9eeea); no conflicts, no merge commits. Own range vs the corrected parent is unchanged at 508 additions + 32 deletions = 540 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. This PR's semantic review requirements (resolved merge unit, reviewed-unit allocation) remain open and are not affected by this test-only carry. Current-head review 5162591539 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 44d06b378b3c7a95ad2d036fa65d950a5442eb7a (single carry commit, author and message preserved, on parent #7390 c93d15ebc48f48aa96e3a6eed157bb1b18967d0d); no conflicts, no merge commits. Own range vs the new parent is unchanged at 508 additions + 32 deletions = 540 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. This PR's semantic review requirements remain open and are not affected by this test-only carry. Current-head review 5163162674 predates this head movement. Normal CI on the new head: https://github.com/block/buzz/actions/runs/34444699895 (no green claim is made here).

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

Reviewed exact head 4bb3be24d064d2c806e10df75d7dc62523edb385 against base 9b000415f0644010f0f5606a971ac8eb92d63bae as :bot: Jude’s code review agent.

No material defect found. The implementation makes the irreversible boundary truthful and preserves recovery:

  • The pre-invite confirmation says accepted membership changes take effect immediately and survive Stop/failure (mobile/lib/features/channels/compose_bar/helpers.dart:332-366). During media work the action changes from Cancel to Stop remaining, avoiding an undo implication (compose_bar_widget.dart:540-549,1014-1019).
  • Completion is counted only after ChannelActions.addMembers returns from relay-accepted publication (helpers.dart:469-491; mobile/lib/shared/relay/relay_session.dart:311-340,803-841). Relay refusal blocks message publication; generation/source fences stop the remaining invitation prefix after a late acknowledgement without hiding the already-committed member.
  • Text and media incomplete paths retain or restore the draft with revision fencing, so stale work cannot overwrite newer edits (draft_lifecycle.dart:27-75; compose_bar_widget.dart:596-675). Successful additions invalidate membership providers, so retry/revisit rescans the roster instead of blindly replaying the accepted prefix (channel_management_actions.dart:93-120).

Verification on the exact, clean head:

  • Full mobile/flutter test: 2,084 passed.
  • flutter analyze: no issues.
  • Mutation checks proved both the incomplete-completion disclosure and Stop remaining label regressions fail the production-composer test (send_lifecycle_tests.dart:164,173). Mutations were restored and the tree rechecked clean.
  • git diff --check passed; local HEAD, live PR head, and base/merge-base were pinned; DCO passed.
  • Verdict-time GitHub snapshot: touched Clients / Mobile and Mobile checks passed; no failing checks. Some unrelated Desktop checks remain in progress.

Confidence gap, not an author defect: no real iOS simulator/VoiceOver/keyboard journey was run. Source inspection keeps the renamed action on the existing keyed InkWell under live progress semantics (upload_progress_pill.dart:82-85,152-190), but that is not native assistive-technology proof. Verification owner: release/native validation if required before merge. Author action: none.

@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 — APPROVE

Reviewed: 9b000415f0644010f0f5606a971ac8eb92d63bae..4bb3be24d064d2c806e10df75d7dc62523edb385.

Risk: high — truthful disclosure around irreversible sequential membership side effects, cancellation/source fences, relay acknowledgements/refusals, draft recovery, and retry.

Findings: no unresolved author-actionable defect. Both lanes independently found the contract truthful and state machine sound:

  • Confirmation discloses before Invite that acknowledged membership persists after Stop/failure.
  • During invitation work, media says Stop remaining, not Cancel.
  • Completion counts advance only after accepted relay OK; refusal prevents publication and reports retained membership/draft recovery.
  • Generation/source fences prevent later work after Stop/revisit/community drift; revision fences prevent stale completion from overwriting newer text.
  • Retry invalidates/re-reads membership rather than blindly repeating an acknowledged prefix.

Author action: none.

Verification owner: CI for exact-head completion; reviewer/release validation for optional native iOS/VoiceOver observation.

Validation: clean exact-head composer suite passed 125; independent full Mobile suite passed 2,084; flutter analyze passed; mutations removing accepted-count disclosure and the Stop-label transition failed the intended production-composer regressions; git diff --check, DCO, base/head freshness, and policy audit passed. Touched Mobile CI was green at final refresh.

Manual/native evidence: no simulator/device/VoiceOver journey. Source and widget evidence establish behavior but are not native assistive-tech proof.

Residual risk: relay timeout remains outcome-ambiguous by pre-existing protocol behavior; this PR truthfully scopes completion to acknowledged invitations. Native focus/keyboard/VoiceOver remains a confidence gap, not author rework. Any new head expires approval.

@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 — APPROVE at exact head a6b55f9199d47e7428864f947bf0127902f525d1 against base 9b000415f0644010f0f5606a971ac8eb92d63bae.

No author-actionable defect was found in the changed-head delta. The irreversible invitation contract remains coherent: membership is counted only after relay OK accepted=true; accepted partial outcomes survive later rejection, Stop, or scope change; community/source/upload-generation/draft-revision fences prevent stale continuation; failed or stopped publication restores only the cleared draft revision without overwriting newer edits; invitation-bearing drafts/media remain until membership preparation succeeds; retry/revisit copy truthfully discloses accepted work and possible reattachment. The original destination is captured before awaits, so recovery does not misattribute work after community drift.

Validation at this exact head with clean worktrees:

  • flutter analyze — PASS.
  • Full mobile/test/features/channels/compose_bar_test.dart — 126/126 PASS.
  • Changed-journey entrypoints (compose_bar_test.dart + channel_detail_page_test.dart) — 343/343 PASS.
  • Causal mutation moving the accepted-outcome callback behind the community fence correctly failed the accepted-scope-switch regression test; mutation was removed and the tree reverified clean.
  • Full mobile run reached 2,083 passes with two timing failures in untouched tests; the affected files passed on isolation/focused rerun, and exact-head Clients / Mobile is green.
  • git diff --check — PASS; merge base matched the supplied base.

The red Desktop and relay-integration leaves fail before product tests/build execution during Chrome apt metadata installation with Hash Sum mismatch. The PR changes only mobile Dart/test files; Windows Desktop build, macOS Desktop build, relay artifact production, and Mobile passed. Those reds are infrastructure noise, not PR-caused gate failures.

Residual confidence gap: no physical-device/screen-reader pass was run in this re-review. Copy, semantics, Stop, retry/revisit, source/revision fencing, draft/media retention, and partial acknowledgement were covered by source and widget tests. Author action: none. Verification owner for any optional device/screen-reader pass: reviewer/release validation, not the author.

Optional follow-up, non-blocking: replace raw relay/channel identifiers in exceptional destination copy with friendly labels while preserving unambiguous source identity.

@loganj
loganj force-pushed the fix/mobile-invitations-20260905 branch from 9b00041 to 1f571bd Compare September 9, 2026 17:52
@loganj
loganj force-pushed the fix/mobile-invitation-commitment-20260909 branch from a6b55f9 to 97147e9 Compare September 9, 2026 17: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 — APPROVE exact head 97147e9a9f0aae7421067ca121d34701cda7d224 over 1f571bd3d74eedee2a0c2dbbdf0a402698c31398. Both lanes re-reviewed the semantic rebase/integration: acknowledged membership is counted before later scope fences; prepare/final authorization, visit/revision/config and upload-generation fences preserve exact intent; actual delivery is tracked separately from irreversible invitation completion; Stop remaining, retry/revisit, source disclosure, and draft/media cleanup remain truthful. Exact-head evidence: full Mobile 2,132/2,132, changed entrypoints 358/358, composer 141/141, analyzer and diff-check pass; suppressing the accepted callback causally fails the irreversible-prefix regression. Author action: none. Verification owner: CI/release for restarted checks and optional physical-device/VoiceOver evidence. Ambient Chrome apt hash failures are not author rework.

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

Reviewed exact head 97147e9a9f0aae7421067ca121d34701cda7d224 against base 1f571bd3d74eedee2a0c2dbbdf0a402698c31398 as :bot: Jude’s code review agent. This review replaces my stale prior-head approval.

No material defect found in the semantic delta or its composition with the new base authorization state machine.

  • Authorization prepare/currentness checks now precede invitation start and membership writes; upload generation, authorization visit/revision, and config identity fence the asynchronous boundaries (mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:483-530,584-608,634-719). Cancellation preserves newer drafts without producing a false access error.
  • Each relay-acknowledged kind-9000 add is recorded before any subsequent continuation-scope check (mobile/lib/shared/actions/channel_management_actions.dart:101-125). Thus a late community/visit cancellation stops remaining work and message publication without erasing the irreversible accepted prefix.
  • Text and attachment paths separately retain actual delivery outcome and report incomplete irreversible effects (draft_lifecycle.dart:22-76; compose_bar_widget.dart:654-729). Source destination is captured before async work, cleanup remains ownership/revision/generation-fenced, and retry/revisit disclosure plus Stop remaining remain truthful.

Exact-head evidence on clean checkouts:

  • Full mobile suite: 2,132/2,132 passed.
  • flutter analyze: passed.
  • Changed entrypoints (compose_bar_test.dart + channel_detail_page_test.dart): 358/358 passed; affected composer suite separately passed 141/141.
  • Mutation suppressing production onAccepted failed the accepted-scope-switch regression at send_lifecycle_tests.dart:191, proving the test detects lost irreversible invitation evidence. A second ordering mutation also failed the affected scope-switch case. Both were restored and trees rechecked clean.
  • git diff --check passed; live head/base and clean state were rechecked after validation; DCO passed.

CI note: Mobile and Mobile Swift are still running. Current Desktop/relay-integration failures occur in the pre-product Chrome apt-install hash-mismatch path and are not causal to this mobile-only delta. Verification owner: CI/infrastructure for those restarted checks. Author action: none.

Residual confidence gap: no physical-device/VoiceOver pass was run for this delta; copy, semantics, and state transitions were source/widget validated. Verification owner: native/release validation if physical-device evidence is required before merge.

@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

Reviewed: 069c8b71a4b43edc3dae946e14d2523fc8a16d29..2943008ec56589140cccd37f507a1393e64a4671 (exact live head rechecked before submission).

Risk: high — the change governs irreversible channel-membership writes across asynchronous authorization, upload cancellation, relay backpressure, and message delivery.

Blocking finding

[P1] Carry the composer attempt fence through rate-limit waiting to the actual invitation enqueue.

The composer checks ensureCurrent() and then directly awaits channelActions.addMembers(...) (mobile/lib/features/channels/compose_bar/helpers.dart:471-480). ChannelActions.addMembers installs only _ensureCommunityValid around publication (mobile/lib/features/channels/channel_management_actions.dart:105-118). The final post-backpressure check in RelaySessionNotifier.publish executes whatever publication guard is in the Zone (mobile/lib/shared/relay/relay_session.dart:356-364), but production never wraps the composer call with withRelayPublicationGuard(ensureCurrent, ...).

Consequently, after the user consents, a rate-limit pause can hold kind 9000; editing/revisiting/unmounting or pressing Stop remaining can invalidate the composer attempt without changing community; when capacity resumes, the stale invitation still enqueues and may irreversibly add the member. The nested test manually supplies the missing outer wrapper (mobile/test/shared/relay/relay_enqueue_fence_test.dart:104-146), so it demonstrates the desired mechanism without proving that the production composer uses it.

Author action: carry the full composer currentness fence through channelActions.addMembers to the actual enqueue (for example, wrap the production call in withRelayPublicationGuard(ensureCurrent, ...)). Add a production-composer regression that pauses the real invitation on the rate-limit gate, invalidates visit/draft/upload generation without switching community, releases capacity, and asserts no kind 9000 was sent. Mutation-removing the production wrapper must make that test fail.

Verification owner: author for the patch and causal regression; :bot: Jude’s code review agent for the changed-head delta and affected gates.

Integrated review

The product/UI pass found the disclosure itself sound: the dialog warns before Invite that accepted membership persists after Stop/failure (helpers.dart:332-367); the action becomes Stop remaining after accepted work starts (compose_bar_widget.dart:584-600,1050-1060); and incomplete delivery reports the accepted count, original destination, member-review remedy, and draft/attachment caveat (helpers.dart:575-593). Draft restoration and optimistic-message cleanup also preserve newer edits (draft_lifecycle.dart:21-76; compose_bar_widget.dart:654-735; send_message_provider.dart:97-119). Those strengths do not close the missing enqueue fence above.

Validation

  • Independent full package run: 2,137/2,137 passed at the exact clean head.
  • Another full package run reached 2,136 passed / 1 unrelated activity timeout; the exact failed test passed immediately in isolation. Exact-head Mobile CI is green.
  • just mobile-check / flutter analyze: PASS, no issues.
  • Composer + channel-detail + relay-enqueue suites: 363 passed; focused composer suite: 141 passed; relay enqueue fence suite: 5 passed.
  • UI mutation forcing the action back to Cancel failed the intended production-composer assertion, then was restored.
  • git diff --check: PASS. DCO, Semgrep, zizmor, and all verdict-time GitHub checks are successful or intentionally skipped.
  • Live PR head remained 2943008ec56589140cccd37f507a1393e64a4671; review worktrees were reported clean.

Manual/native evidence: no physical iOS/Android, VoiceOver/TalkBack, or release-artifact journey.

Confidence gaps: native announcement timing, focus after the label transition, snackbar dwell/readability, and extreme text scaling/localization remain unobserved. Author action: none for these gaps. Verification owner: mobile release/manual QA.

Any new head invalidates this verdict until its delta is reviewed.

@loganj

loganj commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Response to review 5162285765 at exact head 2943008ec56589140cccd37f507a1393e64a4671 (2026-09-10).

The blocking finding is confirmed real at this head: helpers.dart:473-479 awaits channelActions.addMembers(...) directly; ChannelActions.addMembers (channel_management_actions.dart:105-118) installs only _ensureCommunityValid; production never wraps the composer call in withRelayPublicationGuard(ensureCurrent, ...). The nested relay_enqueue_fence_test.dart:104-146 supplies the missing outer wrapper manually, so it demonstrates the transport mechanism without proving the production composer uses it — agreed, and we do not claim otherwise.

The composer fence for the invitation path is carried by the identified descendant stack: #7536 at 2b2a5c68111bacfd7de2486296fbc6a265ab0484 installs the outer invitation guard at helpers.dart:508-516. Its production-path falsification lives at #7539 722d1aa4e3411b02894aac187a02d40140d8a482: the invitation row pauses the real invitation at the actual rate-limit gate, invalidates draft/visit/upload generation without switching community, releases capacity, and asserts no kind 9000 was sent; removing the production wrapper (INVITATION_SCOPE mutant) leaks an accepted kind 9000. Qualifications retained: the invitation-scope row combines a real draft change with capacity-aged evidence (not isolated draft-predicate causality); the provider profile-invitation forwarding mutant's first failing assertion is kind9, with the dedicated invitation-binding mutants establishing the accepted9000 leak separately. Those receipts were executed on the reviewed candidate trees (full suites 2137 (#7527) / 2152 (#7534) / 2162 (#7536) / 2188 (#7539) and private 2231, carried through verified byte-identical mobile trees after the Desktop-only changes), not rerun at this public SHA. #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 approvals do not transfer to other heads or PRs.

We propose crediting that identified composition for this finding rather than claiming the synthetic nested test covers this head. These bytes are not at 2943008, and this is not deployed-composition proof. If this merge unit must carry the wrapper standalone, that is an explicit owner policy decision we are asking to be made before bytes are moved or duplicated; no restack, merge, re-approval, or review closure is requested here. The disclosure/Stop-remaining/retained-prefix delivery behavior and the native-manual confidence gaps need no author action per your own verdict.

2026-09-10 mechanical-carry note (post-response): the branch heads referenced above as current (e.g. 2943008ec56589140cccd37f507a1393e64a4671, and descendant stack pins) have since been mechanically docs-carried — this PR now at 84f61093be51f2824debdf4b65e76d12652acfbf, 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.

:bot: Jude’s code review agent — REQUEST CHANGES remains after author-response reconsideration

Reconsidered: unchanged head 2943008ec56589140cccd37f507a1393e64a4671 against base 069c8b71a4b43edc3dae946e14d2523fc8a16d29; author response #7527 (comment) (5612663648, 2026-09-10 03:55:04Z).

The response accurately confirms the defect and identifies a valid descendant implementation, but does not resolve this merge unit or provide a mechanical guarantee that the unsafe state cannot ship.

Why the blocker remains

At this unchanged head, the composer still calls channelActions.addMembers(...) without carrying its draft/visit/upload-generation currentness check through relay backpressure. The author agrees those bytes are absent here and present only in descendant #7536, with the production-composer falsification in descendant #7539.

The live topology is:

  • #7527: 2943008e, base 069c8b71, CLEAN and independently mergeable
  • #7534: base 2943008e
  • #7536: base #7534 head; contains the outer withRelayPublicationGuard(ensureCurrent, ...)
  • #7539: base #7536 head; contains the cited production-path evidence, currently a separate open PR

Repository rules require reviews/checks and squash merges, but do not atomically bind these four PRs or prevent #7527 from landing by itself. Stack ancestry establishes a possible future composition, not a landing invariant. That distinction is the entire trapdoor here.

The product consequence remains concrete: after the UI says Stop remaining, a rate-limited kind 9000 can still enqueue when capacity resumes. Disclosure that completed invitations persist does not authorize a remaining invitation after Stop.

Author action

Choose one:

  1. Put/cherry-pick the production outer invitation guard and causal composer regression into #7527; or
  2. Restack/change the reviewed merge unit so the candidate submitted for approval contains those bytes.

Alternatively, a repository owner may explicitly adopt and mechanically enforce an atomic composed-landing mechanism that makes 2943008e unreachable as a shipped state and guarantees the fixing descendant lands before any build/release can contain this behavior. No such mechanism exists in the current GitHub rules/topology. A declared sequence or descendant test receipt alone is insufficient.

Verification owner: author for changed bytes/restack; repository owner for any atomic-landing policy/mechanism; :bot: Jude’s code review agent for the resulting exact candidate and composition.

Evidence and confidence

  • Live head remains 2943008ec56589140cccd37f507a1393e64a4671; #7527 is OPEN/CLEAN with all exact-head checks successful or intentionally skipped.
  • The #7536 diff does contain the proposed production wrapper. That proves a bounded fix exists, not that #7527 contains it.
  • The prior exact-head code/test evidence remains applicable because source did not change; this reconsideration was topology/policy-only and did not rerun tests.
  • No repository policy found establishes atomic stacked-PR landing. Native mobile/assistive-technology gaps remain non-blocking and require no author action.

Any new head or mechanically enforced landing proposal requires fresh verification. This response ID has now been processed and should not trigger another same-response reconsideration.

@loganj
loganj force-pushed the fix/mobile-invitations-20260905 branch from 069c8b7 to d080ecc Compare September 10, 2026 04:17
@loganj
loganj force-pushed the fix/mobile-invitation-commitment-20260909 branch from 2943008 to 84f6109 Compare September 10, 2026 04:17

@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

Reviewed: d080eccb12c50a79c6d345e295b5beaae22c7ed9..84f61093be51f2824debdf4b65e76d12652acfbf (exact live head/base rechecked immediately before submission).

Risk: high — irreversible channel-membership writes cross composer cancellation, relay backpressure, acknowledgement, draft recovery, and message delivery.

Blocking finding

[P1] Carry the composer attempt fence through rate-limit waiting to the actual invitation enqueue.

The composer checks ensureCurrent() and then directly awaits channelActions.addMembers(...) (mobile/lib/features/channels/compose_bar/helpers.dart:471-480). ChannelActions.addMembers carries only _ensureCommunityValid through publication (mobile/lib/features/channels/channel_management_actions.dart:103-123). RelaySessionNotifier.publish waits for rate-limit capacity and invokes only the guard present in the Zone immediately before socket enqueue (mobile/lib/shared/relay/relay_session.dart:350-384).

Therefore Stop, draft replacement, revisit, unmount, or upload-generation invalidation during backpressure is not rechecked at enqueue unless it also changes community. When capacity resumes, the stale kind 9000 invitation can still transmit even though the source send attempt was stopped or superseded. That makes Stop remaining false at the boundary this PR claims to govern: the message can remain unsent while a remaining membership side effect lands afterward.

The nested transport test manually supplies the missing outer withRelayPublicationGuard (mobile/test/shared/relay/relay_enqueue_fence_test.dart:105-171); it validates the mechanism but does not prove the production composer carries its attempt fence.

Author action: carry the composer ensureCurrent fence through ChannelActions.addMembers to the actual socket enqueue in this merge unit (for example, wrap the production call with withRelayPublicationGuard(ensureCurrent, ...)). Add a production-composer regression that pauses at the real rate-limit gate, invalidates draft/visit/upload scope without changing community, releases capacity, and asserts no kind 9000 event. Removing only the production wrapper must make that test leak kind 9000 and fail.

Verification owner: author for patch and causal regression; :bot: Jude’s code review agent for changed-head source trace, mutation, full Mobile gate, and fresh CI.

Integrated review

The independent product/UI lane found the disclosure and recovery behavior otherwise sound: consent warns that accepted invitations survive Stop/failure (helpers.dart:342-347); invitation work changes the action to Stop remaining (compose_bar_widget.dart:585-596,1077-1085); incomplete delivery reports the acknowledged count, original destination, durable membership consequence, remedy, and attachment caveat (helpers.dart:577-593; draft_lifecycle.dart:19-76; compose_bar_widget.dart:653-719). The renamed action preserves the existing tappable/semantic ownership (upload_progress_pill.dart:82-85,167-188). These strengths do not close the missing enqueue fence.

Exact-head evidence

  • Dedicated clean worktrees pinned 84f61093be51f2824debdf4b65e76d12652acfbf; merge-base and sole parent match declared base d080eccb12c50a79c6d345e295b5beaae22c7ed9. One commit, 16 mobile files, 540 lines churn; git diff --check passed.
  • Full Mobile suite: 2,137/2,137 passed; just mobile-check passed with no analyzer issues.
  • Relay enqueue fence suite: 5/5 passed. This proves nested guard behavior, not the absent production composition.
  • Product mutations removing the incomplete-send disclosure and forcing the action back to Cancel each failed their intended production-composer assertions, then were restored; trees were rechecked clean.
  • Exact-head Mobile, Mobile Swift, DCO, Semgrep, zizmor, and completed platform/aggregate CI jobs are green. One unrelated Desktop smoke shard remained in progress at verdict time; it does not affect this source-proven blocker.

Confidence gaps, not additional author defects: no physical iOS/Android or VoiceOver/TalkBack journey was run, so OS-level announcement timing and narrow-device visual fit remain unobserved. Author action: none for those gaps. Verification owner: mobile release/manual QA.

Any new head invalidates this verdict until its delta is reviewed.

@loganj
loganj force-pushed the fix/mobile-invitations-20260905 branch from d080ecc to 5f8744a Compare September 10, 2026 05:30
@loganj
loganj force-pushed the fix/mobile-invitation-commitment-20260909 branch from 84f6109 to f098f9a 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 — REQUEST CHANGES

Reviewed: 5f8744a3fb571b6dd9a6d3853ae254694ab9eeea..f098f9aa877235320dade5a76ae1b2ac02e4fe70 (exact live head/base rechecked immediately before submission).

Risk: high — irreversible channel-membership writes cross composer cancellation, relay backpressure, acknowledgement, draft recovery, and message delivery.

Blocking finding

[P1] Carry the composer attempt fence through rate-limit waiting to the actual invitation enqueue.

The composer checks ensureCurrent() before and after directly awaiting channelActions.addMembers(...) (mobile/lib/features/channels/compose_bar/helpers.dart:471-480). ChannelActions.addMembers carries only _ensureCommunityValid through publication (mobile/lib/features/channels/channel_management_actions.dart:102-123). RelaySessionNotifier.publish waits for rate-limit capacity and invokes only the Zone guard immediately before socket enqueue (mobile/lib/shared/relay/relay_session.dart:350-384). A scoped search of mobile/lib found production withRelayPublicationGuard calls in ChannelActions.addMembers and SendMessage.call, but not around this composer membership call.

Therefore Stop, draft replacement, revisit/unmount, or upload-generation invalidation during backpressure is not rechecked at enqueue unless community also changes. When capacity resumes, the stale kind 9000 invitation can still transmit although the source attempt was stopped or superseded. That makes Stop remaining false at the governed boundary: the message can remain unsent while a remaining membership side effect lands afterward.

The nested relay test manually supplies the missing outer guard (mobile/test/shared/relay/relay_enqueue_fence_test.dart:104-171). It proves nested transport fencing works, but does not bind that fence into the production composer path.

Author action: carry composer ensureCurrent through ChannelActions.addMembers to actual socket enqueue in this merge unit (for example, wrap the production call in withRelayPublicationGuard(ensureCurrent, ...)). Add a production-composer regression using the real rate-limit gate: pause invitation publication, invalidate draft/visit/upload scope without switching community, release capacity, and assert zero kind 9000 events. Removing only that production wrapper must leak kind 9000 and fail.

Verification owner: author for patch and causal regression; :bot: Jude’s code review agent for changed-head trace, mutation proof, full Mobile, and fresh CI.

Integrated review

The independent product/UI lane found the disclosure otherwise strong: consent warns that accepted membership survives Stop/failure; acknowledged additions are counted before later scope checks; incomplete delivery identifies the original destination and recovery steps; and active invitation work changes the media action to Stop remaining. The existing InkWell remains the single actionable semantics owner (mobile/lib/features/channels/compose_bar/upload_progress_pill.dart:167-188). Those strengths do not close the enqueue race.

Exact-head evidence

  • Full Mobile: 2,137/2,137 passed.
  • just mobile-check: 558 files unchanged by formatting; analyzer found no issues.
  • Composer + channel-detail + relay-enqueue entrypoints: 363/363 passed.
  • Mutation deleting the inner ChannelActions guard failed the relay cancellation/nesting rows, then was restored. That causally proves the inner community fence, not the missing outer composer fence.
  • git diff --check passed; dedicated worktrees were clean; local/live head, base, sole parent, and merge-base matched. One signed commit; repo-policy preflight passed.
  • DCO, Semgrep, zizmor, Mobile Swift, and completed aggregate/platform checks were green. Exact-head Clients / Mobile and several Desktop jobs remained in progress at verdict time; they do not alter this source-proven blocker.

Confidence gap, not another author defect: no physical iOS/Android or VoiceOver/TalkBack journey was run, so native announcement/focus timing, extreme text scale, and narrow-device fit remain unobserved. Author action: none. Verification owner: mobile release/manual QA.

Any new head invalidates this verdict until its delta is reviewed.

Signed-off-by: Logan Johnson <loganj@squareup.com>
@loganj
loganj force-pushed the fix/mobile-invitations-20260905 branch from 5f8744a to c93d15e Compare September 10, 2026 06:19
@loganj
loganj force-pushed the fix/mobile-invitation-commitment-20260909 branch from f098f9a to 44d06b3 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.

:bot: Jude’s code review agent — REQUEST CHANGES

Reviewed: c93d15ebc48f48aa96e3a6eed157bb1b18967d0d..44d06b378b3c7a95ad2d036fa65d950a5442eb7a (exact live head rechecked immediately before submission).

Risk: high — irreversible channel-membership writes cross composer cancellation, relay backpressure, acknowledgement, draft recovery, and message delivery.

Blocking finding

[P1] Carry the composer attempt fence through rate-limit waiting to the actual invitation enqueue.

The composer calls ensureCurrent() before and after directly awaiting channelActions.addMembers(...) (mobile/lib/features/channels/compose_bar/helpers.dart:471-480). ChannelActions.addMembers carries only _ensureCommunityValid through publication (mobile/lib/features/channels/channel_management_actions.dart:102-125). RelaySessionNotifier.publish waits for rate-limit capacity and invokes only the Zone-carried guard immediately before socket enqueue (mobile/lib/shared/relay/relay_session.dart:350-384). A scoped search of mobile/lib found production withRelayPublicationGuard wrappers in ChannelActions.addMembers and SendMessage.call, but not around this composer membership operation.

Therefore pressing Stop remaining, editing/revisiting, unmounting, or invalidating upload generation while kind 9000 is waiting for capacity does not recheck composer attempt scope unless community also changes. When capacity resumes, a stale invitation can still enqueue although the message remains unsent. This makes Stop remaining false at the boundary this PR claims to govern.

The nested relay test manually supplies the missing outer guard (mobile/test/shared/relay/relay_enqueue_fence_test.dart:138-146), so it proves the transport mechanism but not production composition.

Author action: carry composer ensureCurrent through ChannelActions.addMembers to actual socket enqueue in this merge unit—for example, wrap the production call with withRelayPublicationGuard(ensureCurrent, ...). Add a production-composer regression using the real rate-limit gate: hold invitation publication, invalidate draft/visit/upload scope without changing community, release capacity, and assert zero kind 9000 events. Removing only the production wrapper must leak kind 9000 and fail.

Verification owner: author for patch and causal regression; :bot: Jude’s code review agent for changed-head trace, mutation proof, full Mobile gate, and fresh CI.

Integrated review

The independent systems and product/UI passes agree on the blocker. They also found the surrounding disclosure and recovery behavior otherwise sound: consent warns that accepted membership persists after Stop/failure (helpers.dart:338-347); accepted additions are counted before later continuation checks (channel_management_actions.dart:107-124); incomplete delivery identifies count, original destination, durable consequence, remedy, and attachment caveat (helpers.dart:580-592); and the existing single actionable control changes to Stop remaining without adding duplicate semantics ownership (upload_progress_pill.dart:140-188). Those strengths do not close the enqueue race.

Exact-head evidence

  • Both independent lanes used clean worktrees pinned to 44d06b378b3c7a95ad2d036fa65d950a5442eb7a; live head/base and merge-base matched at report time.
  • git diff --check: PASS.
  • Affected entrypoints—compose bar, channel detail, and relay enqueue suites: 363/363 passed.
  • Focused relay enqueue suite: 5/5 passed; this confirms inner/nested fencing, not the absent production seam.
  • Exact-head Mobile CI is green. At verdict time Desktop Domain/Core and four Desktop smoke shards were still running; those CI-owned checks do not alter this source-proven blocker.
  • DCO, Semgrep, and zizmor are green.

Confidence gap, not author rework: no physical iOS/Android, VoiceOver/TalkBack, extreme-text-scale, or narrow-device journey was run. Native announcement timing, focus, snackbar readability, and layout remain unobserved. Author action: none. Verification owner: mobile release/manual QA.

Any new head invalidates this verdict until its delta is reviewed.

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