Repository navigation
fix(mcp): preserve current attachments across edits - #2937
Merged
Merged
Conversation
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>
bradseiler
force-pushed
the
honor/mcp-edit-attachments
branch
from
November 8, 2026 15:39
eec0c8f to
5b583b8
Compare
bradseiler
marked this pull request as ready for review
November 8, 2026 15:46
kazemek21
approved these changes
Nov 8, 2026
kazemek21
left a comment
kazemek21
left a comment
Contributor
There was a problem hiding this comment.
🤖 Kyle reviewed this PR and the Nexus review-tracker notes and approved with no comments.
Posted by Codex (gpt-6.1-sol) on behalf of kylez.
bradseiler
added a commit
to block/buzz
that referenced
this pull request
Nov 8, 2026
## Summary Editing attachment messages through `buzz messages edit` can remove their files from the effective message. Preserve the latest live imeta set across successive edits and add `--clear-attachments` for explicit removal. Add an SDK builder for complete replacement attachment sets while preserving the existing API. The relay provides a versioned effective-edit lookup on `/query` for kinds 9/40002/40008/45001/45003; media reads fold the same set. It uses the existing normalized-target index, fresh channel authorization, and a primary snapshot. Generic `#e` matching is unchanged. Mixed-case historical edits cannot restore removed files, including when another reference would allow republication. ### Related issue Fixes #3022. Follows #9077. Related open attempts: #3062 and #3065. ### Testing Published head: `c630c4ebf0b20a9618f4844629eec1e666699cbe`, through all mandatory push hooks. The governed-five snapshot passed all four PostgreSQL/HTTP acceptance tests with `RUST_MIN_STACK` unset and 3,333 affected-package tests. The committed test-only array correction passed clippy and the preflight rerun. Independent mutations detected dropped read/lookup kinds, the CLI two-kind split, and preflight ordering, channel and live-edit errors. The published follow-up adds forum preview retention/suppression and a deleted-original preflight fixture; both surviving mutations now fail as intended. Independent review verified the fixes. All four acceptance tests pass; the full DB/relay run had two NIP-FI failures that passed in isolation. The actual standalone MCP workflow passed against this relay and clean local app `5b583b8c69596353bc37e9f3938eca1aabcd081e`: 15 MCP edits, 10 direct publications and 60 member GET/HEAD checks across five isolated blob pairs. Both tested heads are now published. Relay hosted checks report 80 successful and 31 skipped, with none pending or failed; app hosted checks and human upload → edit → clear → edit → GET/HEAD confirmation remain pending. No human acceptance is claimed. ### Rollout The lookup, CLI preservation/clear and media reads use kinds 9/40002/40008/45001/45003. Deploy the relay before releasing the CLI and paired [MCP producer](block/buzz-app#2937). New clients abort without publishing against an unsupported relay. Old CLI edits emit empty imeta and now revoke diff/forum attachments that previously remained readable. Other channel-tagged kinds retain the historical plain-edit path. No additional schema change is needed beyond migration 0086. Readiness remains blocked by mobile edits that omit imeta, the app UI fold that can restore older files, unaudited SDK/direct callers and unmeasured historical exposure. Run `docs/sql/private-media-edit-preflight.sql` for each community; producer updates do not repair signed historical edits. No enforce activation is authorized. `off` skips policy checks, `shadow` changes policy metrics while allowing policy failures, and `enforce` denies inaccessible references. Observe `buzz_media_publication_access_total{mode="shadow",reason="denied"}`. Prefer rolling back the client release while retaining the additive lookup. A relay rollback makes new CLI/MCP edits fail closed; already-published imeta remains valid under the older reference rules. Concurrent edits still use `(created_at,id)` without compare-and-swap. Generated with Codex --------- Signed-off-by: Brad Seiler <seiler@squareup.com> Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz>
bradseiler
force-pushed
the
honor/mcp-edit-attachments
branch
from
November 9, 2026 22:52
5b583b8 to
b68e680
Compare
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>
bradseiler
force-pushed
the
honor/mcp-edit-attachments
branch
from
November 10, 2026 04:30
b68e680 to
b206249
Compare
damienrj
added a commit
that referenced
this pull request
Nov 10, 2026
…rminal-font-picker * 'main' of https://github.com/block/buzz-app: (168 commits) Unify Membership sharing and live roster updates (#3147) fix(activity): show one working cue per agent request (#3166) Introduce opt-in first-agent setup (#2804) fix(codex): drop the security-review sign-in prompt from harness defaults (#3188) feat(releases): publish desktop download manifest (#3154) fix(mcp): preserve current attachments across edits (#2937) fix(agents): call the Claude and Codex first-run download "the integration" (#3165) chore(goose): bump bundled goose-acp to v1.58.1 (#3161) Revert "ci: use larger GitHub runners for Rust and browser journeys (#3090)" (#3180) Show Goose's unset thinking level as Default, not Off (#3124) ci(release): publish stable and beta desktop releases from v tags (#3150) fix(messages): end the New messages hold when no repair can sample (#3169) fix(search): make Command F search the Bestie conversation (#3084) fix(plugins): keep failed update checks on the plugin's Settings row instead of a toast (#3088) Show single-agent Stop on the activity bubble (#3140) fix: support explicit hosted browser origins (#3149) fix: keep Codex tool namespace descriptions bounded (#3128) fix(messages): show "New messages" in channels longer than one page (#3071) Keep shared agent guidance generic (#3134) fix(typing): a thread reply also ends the agent's channel-level typing (#3148) ... Signed-off-by: damienrj <damien@squareup.com> # Conflicts: # src-tauri/Cargo.toml
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
Real standalone MCP workflow passed at app
5b583b8c69596353bc37e9f3938eca1aabcd081eand relayc630c4ebf0b20a9618f4844629eec1e666699cbe. All five supported kinds retained replacement attachments and preserved an empty set after removal. The run recorded 15 accepted MCP edits and 60 member GET/HEAD checks with isolated blobs; unsupported kinds were denied. No browser journeys were added.The full local run at prior app head
5b583b8c69596353bc37e9f3938eca1aabcd081epassed 19,747 tests with 22 skipped, including all 177 broker tests. The comparable pre-fix run passed 19,427 tests with 22 skipped. An earlier run ateec0c8f1ehad five broker failures (localhost connection timeout, presence timeout and subsequent origin failures) and six aborts. Those failures did not reproduce in the isolated run or either uninterrupted comparison; their root cause remains unconfirmed. No assertions or timeouts were weakened.Rebased onto main
bd344a52d18523f4939ee3cef1db43081ef4d89catb206249ecb4f151703f2e56458b2b6900b32c0df. All three patches are equivalent in range-diff, with no conflicts. Main's #3144 repairs the two Activity navigation selectors that failed on the prior head. No extra product or test changes were needed. The fullfocus-navigation.spec.mjsfile passed all 22 cases across Chromium and WebKit at this head (37.9s locally). Normal pre-push types, related unit tests and design checks passed. Current-head CI passed on tested merge0b6109462506e5a7922ebb3415fe7dc508bcc22f(parents: mainbd344a52d18523f4939ee3cef1db43081ef4d89c, PR headb206249ecb4f151703f2e56458b2b6900b32c0df). Independent code/rebase review found no issues; final disposition after CI is pending. Prior paired MCP workflow evidence remains historical; the effective-edit patches are unchanged. Human acceptance remains pending; the existing ready state is preserved.Bigger picture
This is the app producer counterpart to relay PR #9226. Release after that relay contract deploys. Human acceptance remains required before merge. Other app UI/mobile producers and historical media exposure remain separate relay enforcement gates.
Generated with Codex