Skip to content

fix(mobile): authorize all selected mentions from fresh evidence - #7534

Open
loganj wants to merge 1 commit into
fix/mobile-invitation-commitment-20260909from
fix/mobile-classification-consumer-5f397629
Open

loganj wants to merge 1 commit into
fix/mobile-invitation-commitment-20260909from
fix/mobile-classification-consumer-5f397629

Conversation

@loganj

@loganj loganj commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Authorize every selected mention using fresh ordinary/agent/unresolved-agent evidence, both for policy selection and membership roles. Saved isAgent is denial-only taint, never permission: current agent evidence overrides saved false, and lost saved-true provenance stops effects without silently downgrading roles or shrinking recipients.

Reuse authorizeAgentMentions as the sole policy evaluator. Re-read after consent, before individual membership writes and before publication; fence observed profile revision, account/session, draft/visit and upload ownership. Selected evidence reads never populate profile caches. Preserve accepted-ACK receipts before post-write fences and truthful draft/media recovery.

Equivalent relay configuration refreshes preserve scope; actual relay or credential changes still cancel. Includes ordinary member/nonmember, fresh/denied/unknown agent, incomplete evidence, consent/per-write/revision/policy change, accepted-prefix/cancellation and equivalent/changed-config boundary tests.

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.
Follow-up to #7387 and #7531, serially based on #7527 (fix/mobile-invitation-commitment-20260909, exact 97147e9a9f0aae7421067ca121d34701cda7d224), not main. No existing consumer PR found in head/base queries before creation. Also addresses equivalent-config cancellation reported on #7394. Ancestors are unchanged; this PR's own diff is 460 additions + 55 deletions = 515 lines.

Testing (historical candidate receipts)

At substantive consumer HEAD 6513906f9efbc6be9075335e5b5a0a60aa088692:

  • just mobile-check: PASS (format/analyzer).
  • cd mobile && flutter test: 2,147 passed.
  • just file-size-check: FAIL (1227 logical widget lines exceeds the 1200 limit). The previous PASS label was incorrect; the saved receipt records failure. Correction is in progress.
  • just ci: attempted, bounded after 65 seconds in workspace Clippy; no repository-wide green claim. No native iOS/Android or live-relay workflow exercised; these are automated widget/provider tests, not native restart evidence. No visual-layout changes or new screenshots.

Separately, a local-only composition includes actual persistence/exact-binding/discovery/authority siblings, this consumer and #7530 correction e9c8be29e2b59080f8e9529a67853d950d8c6a0c. At private 35b9d518b the complete mobile suite passes 2,186 tests, with mobile-check clean. Ten real ComposeDrafts save → disposal → fresh ProviderContainer → restore widget cases pass; replacing fresh policy-key selection with saved-bit trust makes the restarted denied-agent case fail. This cross-sibling restart test patch is a durable private integration artifact, not part of this standalone PR or a substitute for its committed boundary regressions. Its governing-event accessor compatibility resolution is explicitly documented in the integration packet; no private composition is published as a release branch.

Fresh independent review requires changes: observed policy/runtime and absent-profile freshness are not completely fenced, and cancellation does not yet cross the transport rate-limit wait. The private composition does not include #7391; its e9 accessor adaptation is not yet public compatibility. These remain open, not covered by the historical green test counts; no merge/approval or all-feedback closure is implied.

Size correction (historical 85ddfafe candidate)

At 85ddfafe33b4e4fd2ee2f14c80918af25e0fb294, extracted newly introduced selected-mention preparation into a sibling part without deleting tests. Current own diff 460 additions +55 deletions =515. just file-size-check PASS, just mobile-check PASS, full mobile 2147 passed, HEAD pinned in each invocation. This closes the standalone size violation only; F1/F2 and public cross-sibling compatibility remain open. The older size FAIL above is deliberately retained as a correction of the previous false claim.

Published SEND correction — 2026-09-10

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

Workflows helper test carry — 2026-09-10

Head rebased to 42e830f65596198404f529b57bdb52d0998ce3b0 (single carry commit, author and message preserved, on corrected parent #7527 f098f9aa877235320dade5a76ae1b2ac02e4fe70); no conflicts, no merge commits. Own range vs the corrected parent is unchanged at 518 additions + 76 deletions = 594 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.

The carried workflow-local-controls.spec.ts repair addresses this head's public Desktop Smoke E2E shard 4 failures (run 34436599697: "round-trips and reopens structured message-text conditions" failed all three attempts — openTriggerInspector timeout via wrong-target fill, and a mixed-animation-phase geometry retry): the helper now intersects "Message text" with #wf-step-0-text, and waitForAnimations precedes all three operator geometry samples. Causal wrong-target/bound-target and geometry/settled-geometry controls are documented in WORK_LOGS/MOBILE_FEEDBACK_CLASSIFICATION_20260909/GREEN_7539_WORKFLOW_REPAIR.md; the identical subpatch bytes (SHA256 b8c02397…) are carried here via #7531. This is a test-only carry and does not close the open semantic review blockers from 5162791557 (enqueue-boundary evidence binding and production-seam currentness tests); those remain open on this PR. Current-head review 5162791557 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 a22b79fa8c6c2db81f37953e9c0f7b168b1043d6 (single carry commit, author and message preserved, on parent #7527 44d06b378b3c7a95ad2d036fa65d950a5442eb7a); no conflicts, no merge commits. Own range vs the new parent is unchanged at 518 additions + 76 deletions = 594 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 (now also slowing the outgoing trigger pane and asserting the saved payload step id/text and unfiltered message_posted trigger in the form-builder scenario) — mobile production, mobile tests, and API docs are byte-identical old→new. This is a test-only carry and does not close the open semantic review blockers from 5163166388; those remain open on this PR. Current-head review 5163166388 predates this head movement. Normal CI on the new head: https://github.com/block/buzz/actions/runs/34444699935 (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.

:bot: Jude’s code review agent — REQUEST CHANGES at exact head 85ddfafe33b4e4fd2ee2f14c80918af25e0fb294 against base 97147e9a9f0aae7421067ca121d34701cda7d224.

Blocking findings

  1. [P2] Final invitation/publication can use stale or incomplete identity authority.

    mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:499-510 fences only profile events present in observedProfiles; an absent profile has no revision witness. The selected-evidence result also retains no kind 10100 runtime or kind 30177 governing-policy revision. After final authorization, the invitation write at mobile/lib/features/channels/compose_bar/helpers.dart:472-483 and publication path at compose_bar_widget.dart:725-736 can proceed without an atomic relay capability or complete final revision fence.

    Consequently, a key classified as ordinary while no profile exists can gain a verified OA profile before the side effect and still be invited as a human member; similarly, a runtime/policy revision can supersede the final read while publication still proceeds. The current revision tests mutate between explicit reads, not after the last read at the production effect boundary.

    Author action: carry authoritative absence plus profile/runtime/policy head revisions through the result and verify them immediately at every invitation/publication boundary, or use a relay-atomic authorization/effect contract. Add deterministic regressions for absent-profile→OA appearance and runtime/policy change after the final read but before each side effect.

    Verification owner: author implements; reviewer reruns the production-seam tests and mutation-proves the guard.

  2. [P2] Draft/visit cancellation does not cross the relay rate-limit wait before an invitation is sent.

    _addMentionedNonMembers checks currentness and then awaits channelActions.addMembers (mobile/lib/features/channels/compose_bar/helpers.dart:472-483). That reaches SignedEventRelay.submit; RelaySessionNotifier.publish may wait up to 300 seconds in the rate-limit gate (mobile/lib/shared/relay/relay_session.dart:330-359, relay_rate_limit_gate.dart:13-15,62-66) without a caller cancellation/currentness callback. Editing the draft, leaving/revisiting, or cancelling an upload during this pre-send wait can therefore still release a stale kind 9000 invitation. The later check cannot undo that membership side effect.

    Author action: make the pre-send rate-limit wait attempt-cancellable/currentness-aware, check immediately before socket send, thread that fence through membership writes, and add a production-seam regression: active gate → begin invitation → edit/cancel/revisit → release gate → assert zero kind 9000 frames.

    Verification owner: author implements; reviewer reruns the real-gate regression.

  3. The committed regression suite does not currently prove the production profile-revision fence.

    As a mutation check, replacing profilesCurrent() with unconditional true still left the full mobile/test/features/channels/compose_bar_test.dart suite green at 156/156. This means deleting the production guard is not caught, contrary to the repository's regression standard in AGENTS.md:188-192 and TESTING.md:25-31.

    Author action: bind widget coverage to the real userCacheProvider.cacheProfileEvent revision seam after the final read, covering text publication and invitation/upload settlement as applicable, and mutation-prove that removing the guard fails.

    Verification owner: author supplies; reviewer reruns the mutation.

Exact-head evidence

  • just mobile-check: PASS (559 Dart files, analyzer clean).
  • cd mobile && flutter test: PASS, 2,147 tests in an independent clean run. A separate run observed 2,146 passes plus one apparently unrelated missing animation-ready signal; current evidence does not attribute that to this PR.
  • Focused selected-evidence + ComposeBar suites: PASS, 165 tests.
  • just file-size-check: PASS.
  • git diff --check 97147e9...HEAD: PASS.
  • Accepted-prefix accounting and explicit invite-vs-reference consent paths were traced and appear preserved.

Confidence gaps (not additional blockers)

  • No real iOS/Android live-relay journey was exercised. Author action: none for this verdict. Verification owner: native/release gate.
  • Public sibling composition remains unresolved: PR #7530 overlaps identity/policy/cache/ComposeBar files, and PR #7391 overlaps the composer. Author action: publish/rebase the intended stack and run affected full mobile gates before integration. Verification owner: author/integration gate.

The consent and partial-write recovery work is thoughtful. Unfortunately, both stale windows cross authorization boundaries and one can emit an irreversible membership write. The dungeon remains armed.

@loganj

loganj commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

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

Published SEND correction — 2026-09-10

Head 9bcf69fd428a988358302ea6af1a0328ecf000e9; declared base 2943008ec56589140cccd37f507a1393e64a4671; actual own churn 594 including tests, strictly below600. Supersedes historical candidate pins and coverage below/above. Normal CI: https://github.com/block/buzz/actions/runs/34431161984 (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 5162260830 at 9bcf69fd428a988358302ea6af1a0328ecf000e9 (2026-09-10)

Both blocking findings are confirmed real at this exact head: readSelectedMentionAuthorization() (mobile/lib/shared/mentions/selected_mention_authorization.dart:10-30,65-175) returns a projection without a retained presence/absence witness for profile, runtime, policy, and roster heads carried to enqueue; RelayEvidenceClock/retainPublicationEvidence() have no production caller at this head; and profilesCurrent() at compose line 499-510 is mutation-green here (the suite passes with => true). One path correction for the record: the production file is mobile/lib/features/channels/compose_bar/compose_bar_widget.dart, not mobile/lib/components/compose_bar/....

The complete session-bound evidence capability you requested is carried by the identified descendant stack, not by this head. #7536 at 2b2a5c68111bacfd7de2486296fbc6a265ab0484 retains generation/epoch evidence plus signed coordinate presence/absence in the selected reader (:62-91 per-coordinate publication snapshots, :105-115 exact roster, :141-158 runtime/profile/policy wiring, :216-220 returned capability), aggregates it in the composer operation fence (:494-529, where profilesCurrent:497 checks all returned capabilities — the replacement seam, not the superseded profile-only body at 9bcf69f), and threads it through both effect guards (helpers :508-516 invitation fence; RelaySession :368-395 waits for capacity, checks socket generation, then invokes the Zone guard synchronously with no intervening await). This remains locally observed currentness, not server-atomic authorization against unseen remote changes.

Causal production-path proof lives at #7539 722d1aa4e3411b02894aac187a02d40140d8a482: the GUARDED_DELIVERY, READER_BINDING, INVITATION_READER, and INVITATION_SCOPE mutant removals each end in the intended production classification failures (removing only the composer delivery wrapper leaks kind9 while the transport wrapper remains; invitation counterparts leak accepted kind9000). Qualifications retained: the provider profile-invitation forwarding mutant's first failing assertion is kind9, with the dedicated invitation-binding mutants establishing the accepted9000 leak separately; the invitation-scope row combines a real draft change with capacity-aged evidence; the waiting-revocation row is combined evidence, with the no-capacity signed-boundary row supplying independent coordinate isolation. Those 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 trees after the Desktop-only changes), not freshly 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.

This is conditional cross-PR coverage: not these bytes at 9bcf69f, not standalone authorization completeness, not deployed-composition proof. If this merge unit must be standalone-safe, that is an explicit owner policy decision we are asking to be made before bytes are duplicated or moved; 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. 9bcf69fd428a988358302ea6af1a0328ecf000e9, and descendant stack pins) have since been mechanically docs-carried — this PR now at a3be0741b301a47d5affbc89105b57cfa381d418, 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.

Review of exact head c78d8f4accb8bbb3610be149fd0733f04383da91 against base 97147e9a9f0aae7421067ca121d34701cda7d224.

The replacement introduces the shape of a connection/capacity witness, but production never consumes it, and the authority snapshot remains incomplete. The prior stale-authorization windows therefore remain open.

Blocking findings

  1. High — connection/capacity evidence is dead code at the final-read → effect boundary. RelaySessionNotifier.retainPublicationEvidence() is added at mobile/lib/shared/relay/relay_session.dart:390-399, but repository-wide search across mobile/lib and mobile/test finds only its declaration. The production fence in mobile/lib/features/messages/widgets/compose_bar_widget.dart:515-522 still checks session identity, pubkey, returned profile entries, upload/draft revisions, and config; it does not retain or validate socket generation or the rate-limit epoch. Since publish() may wait at mobile/lib/shared/relay/relay_session.dart:330-359, a capacity pause or reconnect after the final authorization read can still carry stale evidence into a kind 9000 invitation or final message publication.

    Author action: retain the publication-evidence witness with each fresh authorization read and validate it immediately before every role write and final publication. On epoch/generation change, fail closed and re-read. Add deterministic production-seam regressions for rate-limit activation and socket supersession in this window, covering invitation and publication separately.

  2. High — explicit profile absence and runtime/policy revisions are not witnessed. profilesCurrent() at mobile/lib/features/messages/widgets/compose_bar_widget.dart:499-510 iterates only returned observedProfiles.entries; readSelectedMentionAuthorization() can return an empty profile map (mobile/lib/features/messages/selected_mention_authorization.dart:104-112). Thus absence has no live revision witness. Runtime kind 10100 and governing policy kind 30177 heads are not retained by the composer. The comparison in selected_mention_preparation.dart:27-35 helps only if another authorization read occurs; it does not fence the gap after the final read. An absent profile becoming agent-owned, or a runtime/policy replacement or revocation, can therefore cross the final gap and produce a stale member role or stale recipient authorization.

    Author action: return and retain complete per-key revision evidence, including explicit absence and runtime/policy heads, and validate it immediately before each irreversible effect (or make the effect relay-side revision-checked). Add deterministic final-gap regressions for absence→OA appearance and runtime/policy changes at both invitation and publication boundaries.

  3. High — draft/visit invalidation cannot cancel an invitation stalled inside the rate-limit gate. _addMentionedNonMembers() checks current scope and then awaits channelActions.addMembers() (mobile/lib/features/messages/widgets/compose_bar/helpers.dart:503-513). That path awaits signed kind 9000 publication (channel_management_actions.dart:96-120), while RelaySessionNotifier.publish() waits on a gate for up to 300 seconds and accepts no caller cancellation predicate (relay_session.dart:330-359; relay_rate_limit_gate.dart:13-15). Editing the draft, revisiting, or leaving during that wait can still allow the stale invitation to transmit when capacity reopens; the subsequent scope check is too late to undo it.

    Author action: make the pre-send wait cancellable by the composer scope witness, or revalidate a supplied witness after the wait and immediately before socket send. Add a regression that gates kind 9000, invalidates draft/visit scope, releases the gate, and asserts zero EVENT writes.

Required regression evidence

At this exact head, replacing the entire new retainPublicationEvidence() implementation with () => true left the ComposeBar suite green at 156/156. Independently replacing the production profilesCurrent() guard with => true also left it green at 156/156. Both mutations were restored to a clean tree. The committed tests therefore cannot falsify either production fence, contrary to the repository's guard-mutation expectations (AGENTS.md:188-192, TESTING.md:25-31). The production-seam tests requested above are required, and should be mutation-proved at invitation and publication call sites independently.

Validation and resolved behavior

At the pinned head, reviewers reported:

  • just mobile-check: PASS
  • full just mobile-test / cd mobile && flutter test: PASS, 2147/2147
  • ComposeBar suite: PASS, 156/156
  • focused relay/rate-limit/selected-authorization set: PASS, 60/60
  • just file-size-check: PASS
  • git diff --check: PASS; clean worktrees

Identity/role ownership remains fail-closed, accepted-prefix accounting and error classification remain explicit, and inspected consent, recipient-choice, cancellation reporting, partial-effect disclosure, and accessibility behavior are preserved. Those are good properties, but do not close the stale-authority or stale-invitation windows above.

Non-blocking integration/confidence gaps

  • PR #7530 currently conflicts mechanically in the profile/agent-policy stack. This requires ordering/rebase validation by the integration owner, but is not an independent defect in this verdict.
  • Clients / Mobile was still running when last checked; author action is needed only if a required gate establishes a PR-caused failure.
  • No native iOS/Android live-relay invitation journey was run; native/release validation owns that confidence gap, not the author for this verdict.

Verdict: request changes.

@loganj
loganj force-pushed the fix/mobile-invitation-commitment-20260909 branch from 97147e9 to 2943008 Compare September 10, 2026 02:52
@loganj
loganj force-pushed the fix/mobile-classification-consumer-5f397629 branch from c78d8f4 to 9bcf69f 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

Reviewed base 2943008ec56589140cccd37f507a1393e64a4671 → head 9bcf69fd428a988358302ea6af1a0328ecf000e9 with systems/integration and product/adversarial lanes.

Blocking defects

  1. P2 — authority changes can still cross the final authorization-to-effect window.

    readSelectedMentionAuthorization() reads roster, runtime kind 10100, profile kind 0, and governing policy kind 30177, but returns only a classification/member/policy projection (mobile/lib/shared/mentions/selected_mention_authorization.dart:10-30,65-175). The only subsequent currentness check iterates profile events that were present (mobile/lib/components/compose_bar/compose_bar_widget.dart:499-510), so profile absence, runtime, policy, and roster heads have no retained witness. RelayEvidenceClock exists (mobile/lib/shared/relay/relay_evidence_clock.dart:18-91) but a search across mobile/lib and mobile/test found no production owner or caller; retainPublicationEvidence() likewise has no caller there.

    The last reread/check precedes ChannelActions.addMembers, which may wait at the relay capacity gate before socket enqueue (mobile/lib/shared/actions/channel_management_actions.dart:107-118; mobile/lib/shared/relay/relay_session.dart:352-384). A previously absent profile becoming OA-owned, runtime replacement, policy revocation, or roster change can therefore occur after the read and still produce a stale kind 9000 invitation or kind 9 notification. Re-reading narrows the race; it does not close it. Invitation is an irreversible membership effect.

    Author action: carry a complete session-bound evidence capability—roster, explicit profile presence/absence, runtime, governing policy, socket generation, and rate-limit epoch—to the actual enqueue guard for every invitation and message. If a descendant PR contains that implementation, move the required bytes and tests into this merge unit.

    Verification owner: author implements; review team independently mutation-proves both invitation and message enqueue paths.

  2. P2 — the regression suite does not protect the only production profile-currentness guard.

    At the pinned clean head, the complete ComposeBar suite passed 156/156. Replacing the entire profilesCurrent() body (compose_bar_widget.dart:499-510) with => true left the same suite at 156/156 PASS. The tree was then restored and verified clean. The added revision tests vary injected reader results rather than exercise the production cache/revision-to-enqueue seam. This fails the mutation standard in TESTING.md:25-31 and AGENTS.md:188-192.

    Author action: add production-seam tests that inject signed newer—or previously absent—profile, runtime, policy, and roster evidence after the final read and before each real socket enqueue. Bypassing each production guard must make those tests fail.

    Verification owner: author supplies; review team repeats the mutants.

Verified

  • The base-owned relay enqueue guard runs after capacity waiting and immediately before _socket.send; relay_enqueue_fence_test.dart passed 5/5. The prior rate-limit stale-write issue is not an open defect in this PR.
  • Selected-key classification is all-key and fail-closed; invitation role is derived from reread evidence; accepted invitation prefixes remain recorded.
  • Exact-head evidence from the review lanes: just mobile-check passed (561 formatted, analyzer clean), just file-size-check passed (10/10), ComposeBar passed 156/156 before mutation, git diff --check passed, and Clients / Mobile is green.

Confidence gaps

No native iOS/Android live-relay or assistive-technology journey was run. Author action: none for this verdict. Verification owner: native/release gate. Desktop CI was still in progress when this review was submitted; only a causal required-gate failure would create author action.

The exact head remains unsafe at an irreversible publication boundary, and its key production guard is mutation-green.

@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 exact base 84f61093be51f2824debdf4b65e76d12652acfbf → exact head a3be0741b301a47d5affbc89105b57cfa381d418 with systems/integration and product/adversarial lanes. This replacement is a mechanical carry: its base→head patch-id matches the previously reviewed patch, and the PR-owned executable delta is unchanged. Conclusions below were nevertheless re-established against this exact composition.

Blocking defects

  1. P2 — final authorization is not bound to the irreversible enqueue boundary.

    readSelectedMentionAuthorization() reads roster kind 39002, runtime kind 10100, profile kind 0, and governing policy kind 30177, but returns only projected classification, membership, and policy (mobile/lib/shared/mentions/selected_mention_authorization.dart:66-199). The composer retains and rechecks only profile events that were present (mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:499-535); explicit profile absence and roster/runtime/policy heads remain unwitnessed.

    RelayEvidenceClock is declared (mobile/lib/shared/relay/relay_evidence_clock.dart:18-91) and retainPublicationEvidence() is declared (mobile/lib/shared/relay/relay_session.dart:415-424), but repository-wide search across mobile/lib and mobile/test found no production caller of either in this merge unit. Invitation crosses from the last ordinary check into channelActions.addMembers (helpers.dart:500-513), while publication can reach delivery without installing a complete evidence capability (compose_bar_widget.dart:720-736). RelaySessionNotifier.publish() also waits for capacity before its enqueue guard (relay_session.dart:352-384), so this is a genuine async boundary.

    A profile can therefore change from absent to verified OA-owned—or runtime, policy, or roster can be superseded—after the final read but before kind 9000 or kind 9 enqueue. Mobile can irreversibly invite an agent as member or publish recipients under stale authority. Re-reading narrows the race; it does not close it.

    Author action: carry complete session-bound evidence—roster, explicit profile presence/absence, runtime, governing policy, socket generation, and capacity/rate-limit epoch—to the actual enqueue guard for every invitation and message, or move the required descendant implementation and tests into this merge unit.

    Verification owner: author implements; review team independently mutation-proves invitation and message enqueue paths.

  2. P2 — committed tests do not protect the production currentness seam.

    The exact-head ComposeBar control suite passed 156/156. Replacing the entire production profilesCurrent() implementation (compose_bar_widget.dart:501-510) with => true compiled, and the same suite remained 156/156 PASS. The source was restored and the worktree verified clean. Added classification tests change injected reader results; they do not drive signed newer or previously absent evidence through the production observation/cache-to-enqueue path. This violates the causal regression standard in TESTING.md:25-31 and AGENTS.md:188-192.

    Author action: add production-seam tests that introduce signed profile absence→OA ownership and roster/runtime/policy replacement after the final read but before actual socket enqueue. Independently bypass each production invocation and require behavioral failure.

    Verification owner: author supplies; review team repeats the mutants.

Exact-head evidence

  • Full mobile package: 2,152/2,152 PASS.
  • ComposeBar control: 156/156 PASS; guard mutant: 156/156 PASS, proving the missing causal coverage.
  • just mobile-check: PASS (561 Dart files formatted; analyzer clean).
  • just file-size-check: PASS (10/10 policy tests and production-surface checks).
  • git diff --check: PASS; local/remote HEAD exact and final worktree clean.
  • Stack topology verified as #7527 84f61093… → #7534 a3be0741… → #7536 4660aae4…. Live #7539 is a descendant with expanded Desktop/mobile composition; no descendant correctness was transferred into this verdict.

Confidence gaps

No native iOS/Android live-relay or VoiceOver/TalkBack journey was run. Author action: none for this verdict. Verification owner: native/release gate. Exact-head Mobile CI is green; other jobs were still in progress, while cancelled entries from superseded runs are not attributed as PR defects here. Only a causal required-gate failure creates author action.

The all-selected-key classification and accepted-prefix recovery are sound improvements, but this merge unit’s advertised authority capability remains disconnected from production effects and its only partial guard is mutation-green.

@loganj
loganj force-pushed the fix/mobile-invitation-commitment-20260909 branch from 84f6109 to f098f9a 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
@loganj

loganj commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Smoke E2E shard 4 structured message-text failures — repair carried (2026-09-10)

New head 42e830f65596198404f529b57bdb52d0998ce3b0 carries the reviewed Desktop E2E test repair from #7531 (test-only; mobile production/tests and API docs byte-identical old→new). The carried workflow-local-controls.spec.ts delta (identical subpatch bytes, SHA256 b8c02397…, previously published at #7539) addresses this head's run 34436599697 Smoke E2E shard 4 failures of "round-trips and reopens structured message-text conditions": the trigger-inspector wrong-target fill (helper now intersects "Message text" with #wf-step-0-text) and the mixed-animation-phase geometry retry (waitForAnimations now precedes all three operator geometry samples). Causal wrong-target/bound-target and geometry/settled controls are documented in the referenced repair evidence; the same bytes pass at #7539's head.

This carry does not address the open semantic review blockers from 5162791557 (enqueue-boundary evidence binding; production-seam currentness tests) — those remain open author actions on this PR, untouched by this test-only delta. Head movement requires re-validation of current reviews on the new head per their own terms; normal CI is running and no green claim is made from it.

@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 exact base f098f9aa877235320dade5a76ae1b2ac02e4fe70 → exact head 42e830f65596198404f529b57bdb52d0998ce3b0 with systems/integration and product/adversarial lanes. The delta from the prior reviewed head changes only two Desktop E2E files; the mobile patch-id against its corrected parent remains 3f730b45cb9eed904eb391e72cd94d9f2c2073ba. Both mobile blockers were re-established at this exact head.

Blocking defects

  1. P2 — selected-mention authority is not bound to the actual kind 9000 / kind 9 enqueue boundary.

    readSelectedMentionAuthorization() derives roster kind 39002, runtime kind 10100, profile kind 0, and governing policy kind 30177 state (mobile/lib/shared/mentions/selected_mention_authorization.dart:66-199), but returns only classification, membership, and policy projection. Composer currentness retains and rechecks only profile events that were present (mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:499-535); explicit profile absence and roster/runtime/policy heads remain unwitnessed.

    Exact-head search across mobile/lib and mobile/test finds RelayEvidenceClock only at its declaration and base-owned retainPublicationEvidence() only at its declaration. Neither is production-connected. The final read crosses membership and message delivery (helpers.dart:503-513; compose_bar_widget.dart:642-669,725-736) without installing a complete evidence capability in the existing enqueue guard (mobile/lib/shared/relay/relay_session.dart:350-384). Because publication may wait for capacity before socket enqueue, this is a real async boundary.

    After the final read, an absent profile can become verified OA-owned, or roster/runtime/policy authority can be superseded. Mobile can then emit an irreversible kind 9000 invitation with stale role or a kind 9 notification under stale authority. Re-reading narrows the race; it does not close it.

    Author action: carry complete session-bound evidence—including explicit absence, roster/runtime/policy heads, socket generation, and capacity epoch—into withRelayPublicationGuard for every invitation and message enqueue, or move the required descendant implementation into this merge unit.

    Verification owner: author implements; review team independently mutation-proves kind 9000 and kind 9 enqueue paths.

  2. P2 — committed tests do not protect the production profile-currentness seam.

    At this exact head, the clean ComposeBar suite passed 156/156. Replacing the full production profilesCurrent() body (compose_bar_widget.dart:501-510) with => true compiled, and the same suite remained 156/156 PASS. Source was restored; exact HEAD and a clean tree were rechecked. Added classification tests vary injected reader results rather than drive signed evidence through the production observation/cache-to-enqueue seam. This violates the causal regression policy in TESTING.md:25-31 and AGENTS.md:188-192.

    Author action: add production-seam regressions for signed absent→OA profile evidence and roster/runtime/policy replacement after the final read but before real kind 9000 and kind 9 enqueue. Independently bypass each production guard and require behavioral failure.

    Verification owner: author supplies; review team repeats the mutants.

Exact-head evidence

  • Full mobile package: 2,152/2,152 PASS.
  • ComposeBar control: 156/156 PASS; profile-currentness bypass mutant: 156/156 PASS (negative causal evidence), then source restored clean.
  • Selected-authorization plus relay-enqueue suites: 14/14 PASS.
  • just mobile-check: PASS (561 files formatted; analyzer clean).
  • just file-size-check: PASS (10/10 policy tests plus production checks).
  • git diff --check: PASS; local/live HEAD exact and worktree clean.

Confidence gaps

No native iOS/Android live-relay or VoiceOver/TalkBack journey was run. Author action: none. Verification owner: native/release gate. Exact-head CI remained in progress at submission; no red exact-head gate is attributed here. Superseded-run cancellations are not PR defects. Author action: none unless a required gate establishes a causal failure. Verification owner: CI.

The all-selected-key classification and accepted-prefix recovery remain thoughtful and fail-closed in tested transitions. The advertised authority witness, however, still stops before both irreversible effects, and its only partial production guard remains mutation-green.

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-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 exact base 44d06b378b3c7a95ad2d036fa65d950a5442eb7a → exact head a22b79fa8c6c2db81f37953e9c0f7b168b1043d6 with systems/integration and product/adversarial lanes. The changed-head delta from previously reviewed 42e830f… is confined to Desktop E2E coverage; the mobile base→head patch-id remains 3f730b45cb9eed904eb391e72cd94d9f2c2073ba. Both mobile blockers were independently re-established at this exact head.

Blocking defects

  1. P2 — selected-mention authority is not bound to the irreversible kind 9000 / kind 9 enqueue boundary.

    readSelectedMentionAuthorization() reads roster kind 39002, runtime kind 10100, profile kind 0, and governing policy kind 30177, but returns only projected classification, membership, and policy (mobile/lib/shared/mentions/selected_mention_authorization.dart:66-199). Composer currentness retains only profile events that were present (mobile/lib/features/channels/compose_bar/compose_bar_widget.dart:499-535), leaving explicit profile absence and roster/runtime/policy heads without a witness.

    RelayEvidenceClock is declaration-only at mobile/lib/shared/relay/relay_evidence_clock.dart:18-91; exact-head search across mobile/lib and mobile/test also finds base-owned retainPublicationEvidence() only at its declaration (mobile/lib/shared/relay/relay_session.dart:415-424). Invitation crosses the final read into channelActions.addMembers (mobile/lib/features/channels/compose_bar/helpers.dart:503-513), and message delivery crosses into onSend (compose_bar_widget.dart:642-669,725-736). The actual publication guard runs only later, after the capacity wait (relay_session.dart:350-384), without the authority capability.

    An absent profile can therefore become verified OA-owned—or roster/runtime/policy authority can be superseded—after the final read while mobile still emits a stale-role invitation or stale-recipient message. Re-reading narrows the race; it does not close it.

    Author action: carry complete session-bound evidence—explicit profile absence/presence, roster/runtime/policy heads, socket generation, and capacity epoch—into the existing publication guard for every invitation and message enqueue, or include the descendant implementation that does so.

    Verification owner: author implements; review team independently mutation-proves kind 9000 and kind 9 production enqueue paths.

  2. P2 — committed tests do not protect the production profile-currentness seam.

    The clean exact-head ComposeBar suite passed 156/156. Replacing the complete production profilesCurrent() body (compose_bar_widget.dart:501-510) with => true compiled, and the same suite remained 156/156 PASS in both review lanes. Sources were restored and both worktrees verified clean. The tests vary injected reader results but cannot falsify removal of the real cache/revision guard, violating the causal regression standard in TESTING.md:25-31 and AGENTS.md:188-192.

    Author action: add production-seam regressions that introduce signed profile absence→OA ownership and roster/runtime/policy replacement after the final read but before actual kind 9000 and kind 9 enqueue. Independently bypass each production guard invocation and require behavioral failure.

    Verification owner: author supplies; review team repeats the mutants.

Exact-head evidence

  • just mobile-check: PASS (561 files formatted; analyzer clean).
  • ComposeBar control: 156/156 PASS; profile-currentness bypass mutant: 156/156 PASS, proving the missing causal coverage.
  • just file-size-check: PASS (10/10 policy tests plus production checks).
  • git diff --check: PASS; live/local head exact and final reviewer worktrees clean.
  • Exact-head Clients / Mobile and top-level Mobile: PASS.
  • Desktop Core and some Desktop Smoke shards remained in progress at final review polling. Author action: none unless a required gate establishes a PR-caused failure. Verification owner: CI.

Confidence gap

No native iOS/Android live-relay or VoiceOver/TalkBack journey was run. Author action: none for this verdict. Verification owner: native/release gate.

The classification work is thoughtful, but the authority witness still stops before both irreversible effects, and deleting its only partial production guard remains mutation-green.

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