Repository navigation
fix(mcp): preserve current attachments across edits - #2937
Draft
bradseiler wants to merge 3 commits into
Draft
bradseiler wants to merge 3 commits into
bradseiler wants to merge 3 commits into
Conversation
Resolve the versioned effective edit snapshot as the agent before publishing. Preserve empty replacement sets and fail closed on lookup errors. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Keep the effective lookup contract aligned with the five existing editable message kinds. Cover forum edits and unsupported targets without changing ingest policy. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
Reject missing host lookup support and signed replacements of the wrong kind in regression tests. Document the relay prerequisite, rollout signals, rollback risk, and bounded MCP timestamp wait. Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz> Signed-off-by: Brad Seiler <seiler@squareup.com>
baxen
added a commit
that referenced
this pull request
Nov 7, 2026
## Summary These are fixes for three browser-journey flakes that are failing open PRs which don't touch the code under test. The change is test-only. No browser cases are added or removed, and no assertion is relaxed. The red `Browser journeys (chromium, 3/9)` / `(webkit, 4/14)` shards on most open PRs come from a different problem: `sidebar-geometry.spec.mjs:224` is broken on main. #2945 and #2946 already fix it, so this PR leaves it alone. ### 1. `channel-completion.spec.mjs`: cold dev-server load timed out **Evidence:** `Flake check` on #2935 failed `channel-completion.spec.mjs:7` on both engines with `page.goto: Timeout 15000ms exceeded` on `/tests/fixtures/mentions.html?channels&stream&test-controls` ([chromium](https://github.com/block/buzz-app/actions/runs/42317084302/job/128232679087), [webkit](https://github.com/block/buzz-app/actions/runs/42317084302/job/128232678918)). #2935 doesn't touch that page. **Cause:** this is the same cold load #2776 fixed for `mentions.spec.mjs`. Each test here started its own Vite dev server for the same ~1,600-module fixture. **Fix:** - `beforeAll` builds `mentions.html` once and serves it with `build`/`preview`, using the same config as `mentions.spec.mjs`. - The build keeps the dev server's CSS. Tailwind's optimizer and Vite's CSS minifier each round the 20/14 line-height token to `1.42857`, so line boxes become `19.99998px`. In WebKit that put the popup `4.03125px` above the composer, and the test asserts exactly `4`. I confirmed this locally: it fails 5/5 with either setting left on and passes with both off. Turning both off keeps the geometry the test checked before. ### 2. `profiles.spec.mjs:940`: reopening Members right after closing the profile **Evidence:** `Flake check (webkit, 8/9)` on #2909 failed in repeat 8 of 10 ([job](https://github.com/block/buzz-app/actions/runs/42313241314/job/128219877861)). The trace shows this sequence: 1. `Escape` closes the profile panel. 2. Focus returns to "Manage members of One" within 40ms. 3. `Enter` is pressed 10ms later. 4. The DOM after that `Enter` shows the Profile panel again and no Members dialog, so the next `row.click` times out. **Fix:** after each `Escape`, wait for the address to leave the profile (no `"detail"` in the `#buzz=` hash) before reopening Members. This is a lifecycle barrier, not a delay. One step in this test already waited for the panel to unmount before reopening; the other steps didn't wait at all. **Product follow-up (not fixed here):** the order of a dismissal's history return and a quick reopen matters. - If I delay `history.go` by 300ms, the unpatched test takes a different path: the dismissal's `go(-1)` never runs, and the URL stays on the profile visit under the reopened dialog. - A `browser-history` unit sketch (deferred traversal: `push(channel)`, `push(panel)`, `dismiss([panel])`, then `push(settings)` before the traversal commits) lands on `channel`. The new `settings` visit is undone. Someone who owns navigation should decide whether a push during a pending dismissal return should be deferred or replayed. That is a navigation-history change, not a test fix. ### 3. `timeline.mjs` `edge()`: one wheel gesture stopped short **Evidence:** `Browser journeys (webkit, 6/14)` on #2937 failed `history-prefix.spec.mjs:37` with `wheel reaches timeline edge`, `Received: 199` ([job](https://github.com/block/buzz-app/actions/runs/42320781857/job/128244819852)). The trace shows the one `-3251px` wheel ended with `scrollend` and the geometry then held still, so the poll could never pass. **Fix:** `edge()` repeats the gesture, at most four times, until the edge is reached. Every extra gesture must reduce the distance, so an immobile timeline still fails. `upper()` already follows this rule. The case of already being at the edge is unchanged: it still sends one small wheel so production can start paging. ## Not in this PR, from the same survey of the 40 most recently updated open PRs - `sidebar-geometry.spec.mjs:224` (Drafts & sent row) is broken on main since #2733 + #2728 crossed. #2945 is green and #2946 is in CI; either one fixes it. - `solo-chats.spec.mjs:322` (Share, webkit, on #2927) is being handled in draft #2933. - `layout.spec.mjs:803` on #2916 is already fixed on main by #2750. #2916's merge ref predates it. - #2911 (thread virtualization) and #2918 (per-channel notifications) fail consistently in tests their own changes reach, so those look like regressions for their authors, not flakes. ## Validation All runs were local on macOS with `tests/browser/playwright.ci.config.mjs` and `--no-deps`. These are not hosted CI measurements. - `channel-completion.spec.mjs`, `--repeat-each=5`, both engines: 20/20 passed with the change. Before the CSS settings were added, the built fixture failed WebKit 5/5 (`4.03125`). - `profiles.spec.mjs:940`, `--repeat-each=24 --workers=6`, WebKit: passed. It also passed with `history.go/back/forward` delayed by 300ms, in both engines. - `history-prefix`, `history-loading`, `scroll`, `timeline-setup` and `new-message-boundary` (all callers of `edge()`/`end()`), both engines: 70/70 passed. - Full `history-prefix`, `channel-completion` and `profiles` files, both engines: 43/44 passed. The one failure was `profiles.spec.mjs:6` in WebKit, a test this PR doesn't touch. It then passed 4/4 both with and without this change. **Deferred and remaining gaps:** - I couldn't reproduce the cold-load timeout or the WebKit stopping short locally, so I have no local fail-before for those two. The evidence is the CI traces linked above. - The other `edge()` callers (24 spec files) are left to CI. - A green run doesn't prove these flakes are gone. Signed-off-by: Bradley Axen <baxen@squareup.com> Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
MCP text edits copy the original message's attachments. A later edit can restore a removed file or discard files changed by an earlier edit.
What
Retain the latest live attachment set, including an empty set, through the relay's versioned effective-edit lookup. Both app-hosted and standalone agents use their own access.
How
Verify the signed original and replacement, including their target and channel. Abort before publication on unsupported, failed or invalid lookups. Reuse the bounded notes/canvas wait to date each edit after the observed head.
Risk
Deploy relay support from block/buzz#9226 first, including compatibility for diff messages and forum posts/comments. This client requires the exact
[9,40002,40008,45001,45003]lookup filter; relay and client heads must match. Older relays refuse edits safely. The relay still authorizes every retained file for its signer; denial does not retry with attachments removed. Concurrent writers retain relay ordering without a compare-and-swap guarantee.Rollout
Upgrade each community relay before releasing this client. Every MCP agent edit stops working on relays without the versioned lookup. Observe
buzz_media_publication_access_totaldenials and MCP edit errors during rollout. Rolling back the client restores the original-attachment resurrection bug.Testing
No manual testing against the real relay yet. At
eec0c8f1e170fd873507e2a0612c293f5fc51201, mandatory commit/push hooks and the standalone MCP build passed. Both remaining review mutations now fail their new tests, and the restored MCP/native suite passes all 429 tests.The full local Vitest run had 19,451 passes, 22 skips and five failures in two unchanged broker HTTP fixture files. All 176 tests in those files passed together in isolation; the broad-run gap remains open. Current-head CI, the paired real-relay/private-media workflow, final independent review and human acceptance remain pending. No browser journeys were added; signed adapter fixtures do not establish relay behavior.
Bigger picture
This is the app producer counterpart to relay PR #9226. Release after that relay contract deploys. Remain draft until independent review and human acceptance are complete.
Generated with Codex