Skip to content

feat(mobile): read fresh evidence for every selected mention - #7531

Open
loganj wants to merge 5 commits into
fix/mobile-publication-20260905from
fix/mobile-classification-provider-9d869483
Open

feat(mobile): read fresh evidence for every selected mention#7531
loganj wants to merge 5 commits into
fix/mobile-publication-20260905from
fix/mobile-classification-provider-9d869483

Conversation

@loganj

@loganj loganj commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Add read-only exact-key selected-mention evidence for the forthcoming composer classification fix. Every selected key is read; fresh OA/runtime/bot evidence overrides a saved false bit, and saved true is denial-only taint when provenance disappears. Missing/revoked owned policy remains deny-all. Ordinary membership is not a humanity attestation.

This producer does not authorize publication or invitation writes. The consumer must use the existing authorizeAgentMentions evaluator, explicit consent, ordinary permissions and fresh boundary reads. Saved-isAgent consumer behavior is not fixed by this PR alone.

Dependency: based on #7394 (2fd0d0b) as a serial integration extension of its publication guard, not because the additive provider needs that guard to compile. Reuses tested candidate a7c8b2e5 without semantic redesign. Consumer and private cross-PR persisted-draft validation remain separate gates. No sibling changes backported.

Size: 416 additions + 0 deletions, four files, against #7394.

Related issue

Followup to #7387 and #7394; related explicit-invitation recovery #7390/#7527 and shared authority #7530. Existing open PRs searched; no duplicate selected-mention provider publication found.

Testing

At b4f266d, with HEAD printed in the same invocation:

  • just mobile-check: pass, formatter/analyzer clean.
  • cd mobile && flutter test: confirmation 2,119 passed, including nine signed-evidence provider regressions.
  • First full run: 2,118 passed / one unrelated voice-note temp-directory deletion error. Retained failure evidence; isolated voice_note_recording_test.dart passed 20 tests, then full suite passed without code changes.
  • Original identical producer on fix(mobile): read fresh exact-key agent authorization #7393: 2,104 tests and mobile-check passed. Historical just ci timed out in workspace Clippy; no repository-wide green claim or retry here.

No visible UI change in this producer; no screenshots or native-device journey claimed. These are fake-transport signed-evidence tests, not production restart or persisted-draft consumer coverage. Observer/session revision fencing and per-write checks remain required in the consumer; this API is neither a subscription nor an atomic write guarantee.

Docs correction for review 5162334206 — 2026-09-10

Head carried to 1fe1193a29744cf7bd26e2a138db047169ecad7c (child of 17c3b83c60e106902565d3abd879b7456ade9fa9; declared base #7394 2fd0d0bcc45d987b62668985f0eae0134ccde2e2 unchanged). Every new public member of mobile/lib/shared/mentions/selected_mention_authorization.dart — enum values ordinary/agent/unresolvedAgent, fields kind/isMember, and requiresAgentAuthorization — now documents the reviewer's exact distinctions: ordinary means absence of agent evidence, not a humanity claim; isMember is destination-roster presence only; requiresAgentAuthorization routes evidence evaluation and never grants a write. Docs-only: comment-stripped equality with 17c3b83 (zero executable delta; the single removed line is the reflowed enum declaration), mobile/test subtree byte-identical (4912d92ca208…), hermit dart format reports 0 changed, and the exact-head mobile/analyzer receipts quoted above apply to every unchanged byte. Own range vs #7394 is now 451 additions + 0 deletions across five files (427 at 17c3b83 plus 24 net doc lines; the older 416 figure predates the carried test-only head). Normal CI on the new head: https://github.com/block/buzz/actions/runs/34436598394 (in progress). No behavior change is included or requested; re-audit of the public surface against AGENTS.md:150 is requested on the resulting head.

Workflows helper test repair — 2026-09-10

Head carried to 612a131b24a3a57e2b365de87dc98007e165a17d (one commit on the previous head 1fe1193a29744cf7bd26e2a138db047169ecad7c; base #7394 2fd0d0bcc45d987b62668985f0eae0134ccde2e2 unchanged). Own range is now 473 additions + 5 deletions = 478 lines including tests across seven files; the new own delta is test-only, +22/−5 in exactly two Desktop E2E specs (desktop/tests/e2e/workflows.spec.ts +15/−3 at three first-step fill seams, desktop/tests/e2e/workflow-local-controls.spec.ts +7/−2). All mobile production, mobile test, and API-doc bytes are unchanged old-head→new-head. Exact result blobs: workflows.spec.ts 38e95034f0076c388c37bda6670f668e203cfc02, workflow-local-controls.spec.ts 97f70051c995a634c7138417b79e7a00c93a19b6.

The repair fixes this head's public Desktop Smoke E2E shard 4 failure (run 34436598394: workflows.spec.ts:291 timed out all three attempts at createWorkflow:125 waiting for "This workflow may run often" → Turn on, with three flaky siblings). Root cause, causally reproduced locally under an identical 0.1-rate animation slowdown: the fresh-dialog trigger inspector also labels its input "Message text", and during the AnimatePresence wait-mode swap the unqualified locator fills the outgoing trigger input, silently writing a trigger.filter, which suppresses the run-often warning (production warning requires an unfiltered message_posted trigger) and times out the Turn-on wait; the saved payload also lacks the step text. The repair intersects the label with the first-step #wf-step-0-text control at all three seams and asserts the filled value. GREEN control under the same slowdown resolves TEXTAREA #wf-step-0-text, keeps on: message_posted with no filter in the real saved create_workflow.payload.yamlDefinition, and traverses the original activation Turn-on; RED control reproduces the exact failure class (local default operator is str_contains vs CI's str_ends_with; same causal invariant).

Local provenance on the exact published blobs (one worker, cached toolchain, no retries): full workflows.spec.ts spec 29 passed; workflow unit companions 105/105; companion specs 22 passed / 1 failed — the unchanged Darwin keyboard-template screenshot golden (438 px, same failure signature on the unmodified HEAD; actual PNGs differ 18 px run-to-run, not byte-identical; CI runs Linux where it passes); desktop tsc --noEmit (src scope), exact-config Biome, size/px/pubkey gates, vite build --mode e2e all pass. These are scoped local receipts, not public CI clearance. Independent bounded review found no blocker in the exact 27-line delta (patch SHA256 ea77483c…) with recorded residual limits: the keyboard-template immediate fill keeps the same outgoing-trigger exposure (wrong target would fail fast at the listbox before Save; no CI/local failure observed in that mode — bounded residual, not fixed or unexposed), no clean-install/Linux-golden-parity/whole-Desktop-package claim, and prior approvals do not transfer. Normal CI on the new head is running; no green claim is made from it. Current approvals at 1fe1193a are invalidated by this head movement per their own terms and require delta review. Evidence: WORK_LOGS/MOBILE_FEEDBACK_CLASSIFICATION_20260909/GREEN_WORKFLOWS_HELPER_REPAIR.md, GREEN_WORKFLOWS_HELPER_INDEPENDENT_REVIEW.md, GREEN_NEW_SMOKE4_0446_DIAGNOSIS.md.

Workflow regression coverage addendum — 2026-09-10

Head carried to 893c668e1475997142c27dc661def869f56a848a (one test-only commit on 612a131b24a3a57e2b365de87dc98007e165a17d; base #7394 2fd0d0bcc45d987b62668985f0eae0134ccde2e2 unchanged). This carries the bounded response to current reviews 5163210493/5163212369: the scoped locator repair stays correct, but its committed test lacked causal discrimination of a wrong-target fill under ordinary timing. The new commit is +24/−0 in exactly desktop/tests/e2e/workflows.spec.ts (blob af83102b88aa644b7f9fdfd0760fb6b07c016f70): the existing form-builder scenario now slows only the animations that contain the outgoing trigger input through the real AnimatePresence wait-mode handoff, keeps the unchanged helper fill and value assertion, then parses the actual recorded create_workflow.payload.yamlDefinition to require steps[0].id == step_1, steps[0].text == "Workflow notification", trigger.on == message_posted, and no trigger.filter (saved steps serialize as steps[0].id/text; the reviews' step_1.with.text shorthand is not this serializer's shape).

Provenance on the exact candidate tree 89a1666880253e7b7794ed22a7a133fbb7b3c88d: under the slowed handoff the same first-seam wrong-target mutant fails at the existing toHaveValue assertion instead of passing 1/1, the restored candidate keeps the full workflows.spec.ts spec at 29/29, and fresh exact-tree statics (Biome on both specs, src tsc --noEmit, file-size/px/pubkey checks) pass. The independent bounded delta review found no blocker for this exact 24-line change (GREEN_7531_LOCATOR_REGRESSION_DELTA_REVIEW.md; controlled schedule and finite platform limits disclosed, not a universal determinism claim; unchanged Darwin keyboard-template golden residual stays out of scope). Own range vs #7394 is now 497 additions + 5 deletions = 502 lines including tests; mobile production, mobile tests, and API docs are unchanged old-head→new-head — only the Desktop spec changed. Reviews 5163210493/5163212369 predate this head movement; other open reviews on the chain are not resolved by this test-only change. Normal CI on the new head: https://github.com/block/buzz/actions/runs/34444699456 (no green claim is made from it).

Signed-off-by: Logan Johnson <loganj@squareup.com>

@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

Verdict: APPROVE
Reviewed: 2fd0d0bcc45d987b62668985f0eae0134ccde2e2..b4f266dae1f28982f6c43df4d1b9ffa8f3e54d50 (exact live head b4f266dae1f28982f6c43df4d1b9ffa8f3e54d50)
Risk: high — this is a read-only mobile authorization-evidence producer at a relay membership/agent identity boundary. It performs no invitation, notification, publication, cache, or persistence write.

Behavior/contracts traced: relay-authenticated destination membership; exact-key kind 10100 and kind 0 reads; NIP-OA owner verification; exact owner-policy lookup; fresh-evidence versus saved-state classification; request/currentness bounds; and the explicit producer/consumer authorization seam. This is consistent with Buzz's community-scoped identity and bot-membership model in VISION.md; no UI/accessibility surface changed.

Findings: no blocking or non-blocking code defect found. The newest exact 39002 roster head is signature/coordinate checked and malformed or ambiguous membership fails closed (mobile/lib/shared/mentions/selected_mention_authorization.dart:63-96). Every validated selected key receives bounded exact reads; currentness is checked around each async stage (:51-119, mobile/lib/shared/mentions/agent_policy.dart:8-25,43-71). Fresh OA, signed runtime, or relay bot evidence overrides saved false, while saved true with lost provenance remains denial-only unresolved state (selected_mention_authorization.dart:121-142). Missing owned policy projects deny-all rather than reviving runtime authority (:144-163). The immutable total result remains evidence only; API documentation requires the separate evaluator, consent, invitation permission, per-write scope fencing, and re-read after consent/invitation (:6-9,32-40,164-171).

Author action: none.
Verification owner: consumer/integration owner for evaluator binding, explicit consent, ordinary invitation permission, re-read after consent/invitation, per-write currentness fences, and persisted-draft/restart behavior.

Validation at matching clean HEAD:

  • git diff --check 2fd0d0bcc45d987b62668985f0eae0134ccde2e2...HEAD — pass.
  • just mobile-check — pass: 555 files formatted, 0 changed; analyzer clean.
  • cd mobile && flutter test — pass: 2,119 tests.
  • Focused flutter test test/shared/mentions/selected_mention_authorization_test.dart — 9/9 pass.
  • Two causal mutations independently proved the new suite: disabling fresh knownAgent classification failed saved-false recovery; disabling prior-agent denial taint failed lost-provenance classification. Both were restored; final tree was clean at the exact head.
  • GitHub Clients / Mobile, aggregate Mobile, Android APK coverage, DCO, Semgrep, and zizmor are green. Red Desktop/relay E2E jobs failed during unrelated runner setup on the Chrome apt index hash mismatch before tests; their aggregate result checks are green. This is not PR-caused and requires no author action.

Manual/native evidence: none claimed. This producer has no consumer or visible UI binding.

Residual risk: no live-relay/native restart or cross-PR persisted-draft consumer journey was exercised. That remains a consumer/integration confidence gap, not a defect in this read-only producer.

Signed-off-by: Logan Johnson <loganj@squareup.com>

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:bot: Jude’s code review agent — REQUEST CHANGES on exact head 17c3b83c60e106902565d3abd879b7456ade9fa9 against base 2fd0d0bcc45d987b62668985f0eae0134ccde2e2.

Finding

[P2] Document every new public Dart API member (mobile/lib/shared/mentions/selected_mention_authorization.dart:4,11-12,21)

The repository requires doc comments on new public API (AGENTS.md:147-150), but the new enum values (ordinary, agent, unresolvedAgent), fields (kind, isMember), and requiresAgentAuthorization getter are undocumented. The surrounding type-level prose does not document those individual public members. At this authorization-evidence boundary, callers otherwise must infer whether ordinary asserts humanity, whether roster membership implies authorization, and whether the getter grants a write—precisely the dangerous interpretations the type is designed to avoid.

Author action: add precise /// comments to every new public enum value, field, and getter. Explicitly state that ordinary means absence of agent evidence rather than a humanity claim; isMember is destination-roster presence only; and requiresAgentAuthorization routes evidence evaluation rather than granting write authorization. No behavior change is requested.

Verification owner: author runs the static/mobile gates; this reviewer will re-audit the public surface against AGENTS.md:150 on the resulting exact head.

Integrated review result

The systems/integration pass found no behavioral defect: relay NIP-11 self supplies destination authority; membership and owner-policy evidence are exact-coordinate, signed, bounded, and currentness-fenced; malformed or incomplete evidence fails closed; and this producer performs no cache or persistence write. The product/adversarial pass found the documentation-gate defect above. The only changed-head delta from the prior review is test-only (desktop/tests/e2e/video-attachment.spec.ts:1617-1627), using the existing visible Latest affordance before playback-speed assertions; no product or accessibility defect was found there.

Exact-head evidence

  • git diff --check 2fd0d0b…17c3b83 — pass.
  • just mobile-check — 555 files, 0 formatting changes; analyzer clean.
  • Selected-mention authorization suite — 9/9 pass.
  • Full mobile suite in one clean exact-head lane — 2,119/2,119 pass. A second lane saw one relay_socket_liveness_test timing failure, then 2/2 isolated passes; exact-head Mobile CI is green.
  • just desktop-test — 6,450/6,450 pass.
  • Changed Desktop E2E — pass. Full video spec: 12 direct passes plus one unrelated context-menu retry pass.
  • Exact-head CI — all required checks green; mergeable; no unresolved review threads.

Residual risk

This PR intentionally adds a read-only producer without composer/UI binding. Native/live-relay restart, persisted-draft, consent/invitation, publication, evaluator binding, post-consent re-read, and per-side-effect write fencing remain verification obligations for the consumer/integration change, not additional author rework in this producer PR.

Approval is appropriate once the documentation gate is satisfied and the new exact head is re-reviewed.

Answer review 5162334206: document every new public member of
SelectedMentionAuthorization — the SelectedMentionKind values (ordinary,
agent, unresolvedAgent), the kind and isMember fields, and the
requiresAgentAuthorization getter. The docs state that ordinary is the
absence of agent evidence, not a humanity claim; that isMember is
destination-roster presence at read time only; and that the getter routes
evidence evaluation rather than granting a write. Docs comments only; no
executable or test change.

Signed-off-by: Logan Johnson <loganj@squareup.com>
@loganj

loganj commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Response to review 5162334206 — docs correction delivered at head 1fe1193a29744cf7bd26e2a138db047169ecad7c (2026-09-10).

Every new public member of mobile/lib/shared/mentions/selected_mention_authorization.dart now carries a precise /// doc stating your exact distinctions:

  • ordinary — "No fresh agent evidence for this key. Absence of agent evidence, not a humanity claim: the ordinary membership flow applies."
  • agent — verified NIP-OA profile owner, bot destination-roster role, or signature-verified latest kind:10100 runtime head: "Evidence only, never permission to write."
  • unresolvedAgent — agent evidence not safely established; "Never demoted to the ordinary flow; carries no agent entry or invitation role."
  • kind — "Values state what fresh evidence showed, never permission, role, or humanity."
  • isMember — "Presence in the destination's signed roster at read time only, not a role, eligibility, or authorization claim."
  • requiresAgentAuthorization — "Routing only, never a granted write; eligibility, consent, and invitation fences still apply separately."

Docs-only, no behavior change: the single removed line is the reflowed enum declaration; comment-stripped, the file equals the reviewed 17c3b83c60e106902565d3abd879b7456ade9fa9 byte-for-byte (zero executable delta); mobile/test subtree is byte-identical (4912d92ca208…); hermit dart format reports 0 changed — your exact-head mobile-check/analyzer/full-suite receipts apply to every unchanged byte. Own range vs #7394 is now 451 additions + 0 deletions across five files (427 at 17c3b83 + 24 net doc lines). Static gates re-verified on the exact new head (formatter on the doc'd file, both differential file-size gates, git diff --check; no installs). Normal CI is running on the carried head: https://github.com/block/buzz/actions/runs/34436598394 — DCO and dead-token guards already green. The same correction has been mechanically carried across the reviewed descendant stack (#7390/#7527/#7534/#7536/#7539) with comment-stripped equality and byte-identical mobile tests at every level; current pins are in each PR body's dated carry note. Please re-audit the public surface against AGENTS.md:150 on the resulting head as your verification note proposed.

@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

Verdict: APPROVE
Reviewed: 2fd0d0bcc45d987b62668985f0eae0134ccde2e2..1fe1193a29744cf7bd26e2a138db047169ecad7c (exact live head 1fe1193a29744cf7bd26e2a138db047169ecad7c)
Risk: high — read-only mobile authorization evidence at relay membership and agent-identity boundaries; replacement-head change is documentation-only.

Both assigned lanes completed and agreed that the prior documentation blocker is fixed. Every new public enum value, field, getter, and function now documents its semantics and non-guarantees: ordinary is absence of agent evidence, not a humanity claim; roster presence is not authorization; and evaluator routing does not grant writes (mobile/lib/shared/mentions/selected_mention_authorization.dart:3-72). This satisfies AGENTS.md:150.

The underlying producer remains read-only and fail-closed: destination authority comes from relay NIP-11 self; exact signed kind-39002 membership rejects malformed or ambiguous state (selected_mention_authorization.dart:87-120); runtime/profile/policy reads are bounded, exact-key, and currentness-fenced (:78-143,195-196; agent_policy.dart:8-71); lost provenance stays unresolved and missing owned policy projects deny-all (selected_mention_authorization.dart:145-193). Repo-wide Dart caller search found no production consumer, so enqueue, invitation, persistence, and publication remain downstream obligations.

Author action: none.
Verification owner: consumer/integration owner for composer binding, consent and invitation authority, post-effect rereads, per-side-effect generation fences, enqueue semantics, and persisted-draft/restart recovery.

Exact-head validation on clean trees:

  • git diff --check and differential file-size preflight passed.
  • just mobile-check passed: 555 files unchanged; analyzer clean.
  • Full mobile package passed 2,119/2,119; focused producer suite passed 9/9.
  • A causal mutation removing signed-runtime agent classification failed the intended regression and was restored.
  • Exact-head Mobile, Mobile Swift, builds, integration, DCO, Semgrep, zizmor, and Desktop smoke shards 1–3 passed.

CI confidence gap, not author rework: Desktop smoke shard 4 failed one unrelated workflows.spec.ts responsive-viewport test and marked three other workflow tests flaky after 344 passed. The PR’s replacement delta only documents a mobile public API; neither the documentation delta nor the PR’s carried Desktop video test touches workflows.spec.ts. No PR-caused required-gate failure is established. CI/Desktop maintainers own retry or waiver.

Manual/native evidence: none claimed. No consumer or UI binding exists in this PR.

Residual risk: no native/live-relay, persisted-draft, or consumer publication journey was exercised. Those remain downstream integration work, not defects in this evidence producer. Any new head invalidates this 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 exact head 1fe1193a29744cf7bd26e2a138db047169ecad7c against base 2fd0d0bcc45d987b62668985f0eae0134ccde2e2.

The prior documentation defect is fixed, and the full A Team re-review found no remaining author-actionable defect.

Integrated result

  • Public API gate: every new enum value, field, getter, and function now has precise /// documentation, satisfying AGENTS.md:150. The contract explicitly says ordinary is absence of agent evidence rather than a humanity claim; roster presence is not authorization; and evaluator routing is not a write grant (mobile/lib/shared/mentions/selected_mention_authorization.dart:3-72).
  • Authorization boundary: destination authority still comes from relay NIP-11 self; exact signed kind-39002 membership is selected; malformed or ambiguous state fails closed; runtime/profile/policy reads are exact-key and bounded by currentness checks; invalid or lost provenance cannot fall into the ordinary flow; and missing owned policy projects deny-all (selected_mention_authorization.dart:78-196; mobile/lib/shared/relay/relay_session.dart:152-217; mobile/lib/shared/agents/agent_policy.dart:8-71).
  • Composition: exact ancestry is linear: 2fd0d0b → b4f266d → 17c3b83 → 1fe1193. The replacement delta is limited to documentation plus canonical enum formatting in one file; the mobile test tree is byte-identical across 17c3b83..1fe1193. No descendant or sibling stack work entered this range.
  • Scope: repo-wide Dart search found no production consumer. This PR introduces a read-only evidence producer, not enqueue, persistence, invitation, publication, UI, or accessibility behavior.

Exact-head validation

  • File-size preflight — 10/10 pass.
  • git diff --check 2fd0d0b...HEAD — pass.
  • just mobile-check — 555 files, 0 changed; analyzer clean.
  • Focused selected-mention suite — 9/9 pass.
  • Full mobile package — 2,119/2,119 pass in both independent clean-head lanes.
  • Causal mutation removing signed-runtime classification failed the intended assertion, then restoration returned 9/9 green.
  • Exact-head Mobile, Mobile Swift, macOS/Windows builds, relay integration, DCO, Semgrep, zizmor, and Desktop smoke shards 1–3 passed.

CI confidence gap

Desktop Smoke E2E shard 4 failed after 344 passes: four pre-existing desktop/tests/e2e/workflows.spec.ts cases timed out waiting for the same “This workflow may run often” dialog button; three were classified flaky and one exhausted retries. This PR does not change that workflow test or its production path. The changed video spec is on another path, and the replacement-head delta is documentation-only. This is therefore an unrelated CI confidence gap, not author rework or a basis to preserve a stale change request.

Author action: none.

Verification owner: Desktop/CI maintainers should retry or investigate the workflow-dialog flake before merge according to branch policy. Any new PR head invalidates this approval and requires delta review.

Residual integration risk

Native/live-relay restart, persisted-draft recovery, evaluator/consent/invitation binding, post-consent reread, enqueue semantics, and per-side-effect currentness remain obligations of the later consumer/integration change; this producer has no such side effects.

The outgoing trigger inspector also labels its value input "Message
text", so the shared label could fill the trigger field instead of the
new send_message step during the AnimatePresence wait-mode swap,
silently writing a trigger filter that suppresses the run-often
warning and times out the Turn-on wait. Intersect the label with the
first-step #wf-step-0-text control at all three workflows.spec.ts
seams, assert the filled value, and reuse the already validated
local-controls helper binding and settled operator geometry.

Signed-off-by: Logan Johnson <loganj@squareup.com>
@loganj

loganj commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Workflows.spec.ts Smoke E2E shard 4 — root-cause repair carried (2026-09-10)

Following up on the CI confidence gap flagged in the current reviews (verification owner: Desktop/CI maintainers — no retry or waiver requested): new head 612a131b24a3a57e2b365de87dc98007e165a17d carries a reviewed, test-only repair for the exact seam behind the workflows.spec.ts timeouts at run 34436598394.

Causal diagnosis: in a fresh Create workflow dialog the trigger inspector is open and its condition input is also labeled "Message text"; during the AnimatePresence wait-mode swap after Add step → Send Message, the unqualified locator resolves the outgoing trigger input, silently writing a trigger.filter. The run-often warning requires an unfiltered message_posted trigger, so the "This workflow may run often" alertdialog never appears and the Turn-on wait times out — the saved payload also lacks the step text. Reproduced locally under an identical 0.1-rate animation slowdown (RED: filter written, step text missing, warning suppressed, 30s timeout; GREEN with the fix: #wf-step-0-text textarea filled, no filter, payload verified through actual activation).

Fix: intersect the label with the first-step #wf-step-0-text control at all three workflows.spec.ts seams and assert the filled value. Local receipts on the exact published blobs: full workflows spec 29/29, workflow unit companions 105/105, companions 22 pass / 1 pre-existing Darwin keyboard-template golden (unchanged signature on the unmodified HEAD; CI runs Linux where it passes). Independent bounded review found no blocker in the exact 27-line delta. Known bounded residual (recorded, not fixed): the keyboard-template test's immediate fill keeps the same outgoing-trigger exposure, but a wrong target there fails fast at the autocomplete listbox before Save; no CI or local failure has been observed in that mode.

Older-text purpose, attachments, and all historical receipts in the PR body are preserved. Head movement invalidates the current approvals per their own terms; delta review on the new head is expected. Normal CI on the new head is running — no green claim is made from it.

Addendum — 2026-09-10, head 893c668e1475997142c27dc661def869f56a848a — responding to change requests 5163210493/5163212369. Both are valid missing committed causal coverage; the scoped locator repair above is not wrong. The new head carries a test-only +24/−0 delta in desktop/tests/e2e/workflows.spec.ts (blob af83102b88aa644b7f9fdfd0760fb6b07c016f70): the existing form-builder scenario now slows only the animations that contain the outgoing trigger input through the real AnimatePresence wait-mode handoff, keeps the unchanged helper fill and value assertion, then parses the actual recorded create_workflow.payload.yamlDefinition to require steps[0].id == step_1, steps[0].text == "Workflow notification", trigger.on == message_posted, and no trigger.filter. (Persisted steps serialize as steps[0].id/steps[0].text — the reviews' step_1.with.text shorthand is not this Desktop serializer's shape.)

Under that slowed handoff the reviews' first-seam wrong-target mutant now fails at the existing toHaveValue assertion — the same mutant passes 1/1 under ordinary timing — so the fill discrimination is committed causal coverage, while the saved-payload assertions are independent coverage of what the UI actually persisted (the mutant's first failure does not itself prove a corrupt payload in that run; the historical slowed controls separately established that path). The restored candidate keeps the full workflows.spec.ts spec at 29/29, and fresh exact-tree statics (Biome, src tsc, size/px/pubkey) pass on candidate tree 89a1666880253e7b7794ed22a7a133fbb7b3c88d — blob-identical to the independently reviewed delta (no blocker: GREEN_7531_LOCATOR_REGRESSION_DELTA_REVIEW.md). The schedule is a finite control on the evidenced platform, not a universal determinism claim; the unchanged Darwin keyboard-template golden residual stays out of scope. Normal CI on the new head: https://github.com/block/buzz/actions/runs/34444699456 (no green claim; prior reviews/approvals do not transfer to the new head).

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:bot: Jude’s code review agent — REQUEST CHANGES on exact head 612a131b24a3a57e2b365de87dc98007e165a17d against base 2fd0d0bcc45d987b62668985f0eae0134ccde2e2.

Finding

[P2] Make the workflow-locator regression independently falsifiable (desktop/tests/e2e/workflows.spec.ts:96-102; repeated at :704-710 and :813-819)

The new toHaveValue assertion reads the same locator whose ambiguous binding caused the regression. In exact-head mutation testing, reverting the first seam from the scoped #wf-step-0-text intersection to the old dialog.getByLabel("Message text"), rebuilding, and running creates a workflow via the form builder still passed 1/1. The checked-in test therefore does not deterministically distinguish “the send-message step received the text” from “the outgoing trigger filter received it.” A future timing/animation change can restore the wrong binding while the assertion remains green, leaving the saved workflow with a trigger filter and missing step text. That conflicts with the repository’s falsifiability requirement (AGENTS.md:188-192; TESTING.md:25-31).

Author action: make one checked-in regression deterministic at the outgoing-trigger/step transition (the existing controlled animation-delay seam is suitable), then assert independent saved payload state: step_1.with.text == "Workflow notification" and the message_posted trigger has no filter. Mutation-revert the scoped locator and require that regression to fail behaviorally; restore it and require green. The repeated scoped-locator fixes may remain.

Verification owner: author provides the deterministic regression and mutation receipt; this reviewer re-runs the exact-head test and delta review.

Integrated review result

The systems/integration lane found no defect in the mobile evidence producer: relay NIP-11 authority, exact signed roster selection, bounded runtime/profile/policy reads, scope checks around asynchronous stages, denial-preserving prior-agent handling, and deny-all missing-policy projection remain intact (mobile/lib/shared/mentions/selected_mention_authorization.dart:66-193; mobile/lib/shared/agents/agent_authorization.dart:125-162; mobile/lib/shared/agents/agent_policy.dart:8-71). Repo-wide Dart search found no production consumer or write side effect.

The changed head modifies only Desktop E2E selectors/assertions. The selector fix is sensible, but the product/adversarial mutation demonstrates that its regression proof is not causal. That concrete evidence controls the integrated verdict.

Exact-head validation

  • Exact base/head, clean worktrees, git diff --check, and DCO trailers — pass.
  • just mobile-check — 555 files unchanged; analyzer clean.
  • Selected-mention suite — 9/9; prior-agent denial mutation failed the intended assertion, then restoration was clean.
  • Full mobile runs each saw one different unchanged flaky test; both passed isolated, and exact-head Mobile CI is green. These are confidence gaps, not author defects.
  • just desktop-test — 6,450/6,450.
  • just desktop-check and just desktop-typecheck — pass.
  • Changed workflow E2E paths passed; one unchanged Darwin screenshot golden differed by 438 pixels on retries.
  • Exact-head CI had no failure at final fetch; Desktop Core and smoke shards 2/4 remained pending.

Residual confidence gaps

No live-relay/native-device/persisted-draft consumer journey was exercised because this PR still exposes only the read-only producer. Composer binding, consent/invitation authority, post-consent/invite rereads, enqueue semantics, per-write fencing, and restart recovery remain obligations of the downstream consumer/integration change, not additional author rework 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.

The changed-head implementation is structurally sound in the systems lane, but the Desktop workflow-locator regression added on this exact head is not causally protected.

[P2] The workflow-locator regression remains mutation-greendesktop/tests/e2e/workflows.spec.ts:96-102, with the same pattern at :704-710 and :813-819.

The new assertion reads the same locator whose ambiguous binding caused the reported failure. Reverting only the first changed seam from the scoped #wf-step-0-text intersection to the old dialog.getByLabel("Message text"), rebuilding the E2E artifact, and running creates a workflow via the form builder still passes 1/1. The checked-in test therefore cannot deterministically distinguish the intended send-message step receiving text from the outgoing-trigger filter receiving it. Under ordinary timing, the exact regression can survive while the assertion remains green and the saved workflow contains a trigger filter instead of the intended step text. That violates the repository's production-seam/falsifiability contract in TESTING.md:25-31.

Author action: add a deterministic checked-in regression around the outgoing-trigger/step transition (using the existing controlled animation-delay seam if appropriate), then assert independent saved payload state: step_1.with.text == "Workflow notification" and the message_posted trigger has no filter. Reverting the scoped locator must make that regression fail behaviorally; restoring it must pass.

Verification owner: author for deterministic regression and mutation receipt; A Team for exact-head re-review.

Integrated exact-head evidence: git diff --check, DCO audit, just mobile-check, selected-mention tests 9/9, just desktop-test 6,450/6,450, just desktop-check, and just desktop-typecheck passed. Changed workflow paths passed in the built E2E run; one unchanged Darwin screenshot golden differed. Separate full Mobile runs each encountered a different base-identical flaky test which passed or is green in exact-head CI. Current required checks show no failures, with Desktop Core and smoke shards 2/4 still running. Those are confidence gaps, not additional author defects.

The committed form-builder scenario could not distinguish a wrong-target
"Message text" fill under ordinary timing: the outgoing trigger inspector
keeps its identically labeled input actionable through the
AnimatePresence wait-mode handoff to the first step textarea, so the
repaired locator still needs committed causal coverage. Slow only the
animations that contain the outgoing trigger input, run the unchanged
helper fill and value assertion inside that real handoff window, then
parse the actual recorded create_workflow.payload.yamlDefinition to
require the persisted step_1 id and "Workflow notification" text and an
unfiltered message_posted trigger.

Under this schedule a first-seam wrong-target mutant fails at the
existing value assertion instead of passing 1/1; the restored scenario
and the full 29-test workflows spec stay green. The check reads the
production serializer's own saved YAML (steps[0].id/text), not a
test-authored shape.

Signed-off-by: Logan Johnson <loganj@squareup.com>

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: APPROVE

Reviewed exact head 893c668e1475997142c27dc661def869f56a848a against base 2fd0d0bcc45d987b62668985f0eae0134ccde2e2. No unresolved author-actionable defect found.

The selected-mention authorization producer is fail-closed at the important boundaries: exact roster selection, malformed or ambiguous identity evidence, bounded exact-key runtime/profile/policy reads, freshness checks, lost provenance, and missing owned policy. The public semantics are also appropriately narrow: ordinary does not prove a human, roster presence is point-in-time membership, and requiresAgentAuthorization is routing evidence rather than permission. Eligibility, consent, invitation permission, and scope remain separate downstream checks.

The latest head also repairs the desktop regression test itself. The new scenario targets the step-specific message-text control and independently inspects the recorded create_workflow payload. Mutation validation showed the test fails at the intended assertion when the old ambiguous locator is restored; at exact head the full workflow spec passed 29/29.

Validation at exact head:

  • Full desktop/tests/e2e/workflows.spec.ts: 29/29 passed.
  • Causal locator mutation: targeted scenario failed on all three attempts with empty step text, proving the regression test detects the repaired seam.
  • Mobile selected-mention authorization tests: 9/9 passed.
  • flutter analyze: no issues.
  • dart format --output=none --set-exit-if-changed .: 555 files checked, no changes.
  • git diff --check: passed.
  • GitHub mobile, Mobile Swift, macOS build, integration, security-analysis, and three of four desktop smoke shards were green at final review; Desktop Core and smoke shard 2 remained in progress, with no failure reported.

Confidence gap, not author rework: this PR introduces a read-only producer with no current production consumer, so native-device and integrated mention publication/invitation behavior must be validated when a downstream consumer wires it into the UI. That consumer and its CI/release gate own end-to-end verification, including persisted-draft recovery, consent/invitation checks, post-effect rereads, and per-write fencing.

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