fix(acp): preserve routing for edited messages - #4741
Conversation
a47e638 to
ac89510
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac89510841
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1255ee648
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
4271fc7 to
a0ec518
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0ec518e1f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
a0ec518 to
c2573e3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2573e3909
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12d94766ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fced1e6e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 329af7704d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Review submitted by Carl on Wes's behalf. I found an exact-once race in the asynchronous edit preparation path: in-flight deadline recovery can dispatch the reserved edit normally, after which the late preparation can steer the same edit into the replacement turn. Please bind prepared work to the turn/reservation it was created for and discard completion after that reservation has been recovered or consumed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1eddb28255
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a625232e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6f0fc0c0f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea2542de81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ea2542d to
6b6a033
Compare
|
@codex review |
|
The requested exact-once reservation fix is present on current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b6a033f09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22a0ffdafe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
22a0ffd to
2334ad7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2334ad7ebc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c7f5ac8dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfa6469640
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e03c2c6fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
5e03c2c to
0a70cb7
Compare
loganj
changed the title
81f8baf to
d4ed2d5
Compare
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: REQUEST CHANGES
Reviewed: bbd20fae75ecc3bd7a83cc12a65379fac22a2b79..d4ed2d5d3a49067d61049fff79de62b7ab0d4a78 (exact live head)
Risk: High — this changes ACP messaging identity across async fetch, queue/cancellation, reactions, setup mode, and terminal recovery.
Blocking findings
-
Setup-mode edit routing cannot execute in the live listener.
crates/buzz-acp/src/setup_mode.rs:412-415rejects every kind except stream messages and workflow approvals before mention matching andpublish_setup_nudge, while the default setup rule atsetup_mode.rs:525-528also omits kind40003. Consequently, an eligible edit mentioning an unavailable agent never reaches the new original/root routing code. The helper-only tests atsetup_mode.rs:1065-1140do not cross either production gate.Author action: admit
KIND_STREAM_MESSAGE_EDITin the setup listener and its default subscription, define the intendedkinds_overridebehavior, and add a listener-level regression proving a mentioned edit reaches nudge publication at the original/root. -
Terminal failure notices for edits detach from the visible request. All auth-error, hard-timeout, and retry-exhaustion paths call
spawn_failure_notice, which derives routing by applyingparse_thread_tagsto the raw last event (crates/buzz-acp/src/lib.rs:3996-4010). That parser intentionally ignores the edit's bare targetetag (crates/buzz-acp/src/queue.rs:964-982), sopost_failure_noticereceives no root and publishes at channel top level (crates/buzz-acp/src/pool.rs:4633-4656). An edit-triggered terminal error therefore loses the original-message/thread routing this PR establishes, leaving recovery guidance detached in a busy channel.Author action: carry the resolved edit route into terminal-result handling (or resolve it on that bounded path), route notices to the original ID/root, and add signed-event tag assertions for terminal failure after top-level and threaded edits.
Verification owner: author for both fixes and biting regressions; reviewer for exact-head delta review and mutation/negative proof that bypassing each production route fails its test.
Validation
At clean detached HEAD d4ed2d5d3a49067d61049fff79de62b7ab0d4a78:
git diff --check bbd20fae75ecc3bd7a83cc12a65379fac22a2b79...HEAD— pass.. ./bin/activate-hermit && cargo test -p buzz-acp --all-targets— pass: 828 unit tests and 9 lifecycle tests, 0 failures.- Required GitHub checks — all applicable checks pass at this head.
- Authenticated reviewer
jedwards27; PR authorloganj.
The green suite confirms existing behavior but has no listener-level setup-edit test and no terminal failure-notice edit-routing assertion, so it does not refute either reproduced source-path defect.
Manual/native evidence: none; these are deterministic ACP event-routing contracts best established with Rust listener/signed-event tests rather than UI screenshots.
Residual risk: live relay setup-mode and terminal-failure journeys were not exercised because both source paths are currently defective. Re-run risk-shaped integration evidence after the fixes. Any new head requires delta review.
A kind:40003 edit is an auxiliary event: its bare `e` tag names the edited message, not a thread. When an admitted edit mentions an agent, every reply, reaction, typing indicator, session scope, setup nudge and terminal failure notice must follow the original message. Resolve the original once at admission with a bounded, kind- and channel-scoped relay lookup, and carry the resolved route on the queued and batch events. All routing consumers read that route instead of the edit's own tags. If the original cannot be fetched, route to the edit target id, never to the edit event. Setup mode now admits delivered edits and anchors the nudge at the original's thread root or the original itself. Queued edits dispatch in their own batch, and a cancelled edit returns to the ordinary queue so it keeps its routing boundary. Ported onto current main from the original 4741 stack (old head d4ed2d5), incorporating review feedback. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
d4ed2d5 to
e5d3c4f
Compare
github-actions
Bot
added
the
codex-security-review-current
🔐 Codex Security Review
|
|
Status of the review findings at head Changes-requested review, finding 1 — setup-mode edits never reach nudge publication. Fixed. The setup listener gate ( Finding 2 — failure notices for edits post at the channel top level. Fixed. Codex Security Review — default subscriptions omit kind 40003. Correct for this PR alone, and intentional. This PR makes routing correct for edits when they arrive. It does not change which kinds an agent subscribes to by default. #6131 adds Also fixed during the port: edit-original lookups now include CI: every failure at this head is in code this PR does not touch. The stack changes only |
wpfleger96
left a comment
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 thanks for turning these around. this is a combined review at e5d3c4f2: two independent source passes plus a live run against a local relay with signed events and a scripted ACP peer (edit kinds enabled explicitly, since default wake is #6131).
the two earlier findings are fixed. setup mode admits edits and nudges at the original or its root, and terminal notices follow the resolved route. reverting either production fix makes the new tests fail, so they do catch the old bugs.
two blockers left, details inline:
- interrupting a turn that's working on an edit re-runs the old edit before the replacement, and neither prompt carries the supersede framing. reproduced in the queue-to-prompt path and live.
- when the original lookup fails and the original was a thread reply, the failure notice and the setup nudge claim that reply as the root. the relay rejects both, so the user gets no notice at all.
two nonblocking notes inline too. one more on the description: without #6131, setup mode still won't see edits under the default subscription, so "Setup mode now accepts edits" is only true for the stack. fine as long as #4741 and #6131 land together.
what held live: top-level, threaded, nested and kind 40002 originals, thread-scoped sessions, 👀/💬 on the original, resolved-thread failure and setup notices, relay rejection of edits from non-authors, and ordinary-message interruption.
…oots An interrupted edit now merges into the replacement turn as prior work, so the replacement gets supersede framing and the old edit never re-runs. Edits no longer get their own batch; routing already rides on each event. When an edit's original is unresolved, the failure notice retries the lookup. If it is still unresolved, the notice and the setup nudge post at top level instead of claiming the target as a root the relay would reject. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
github-actions
Bot
removed
the
codex-security-review-current
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: 0ee609379004a894e99b449885c37a044f41919e...e079ba645646ddcb7fe35cb74b6c4418a0718132 (exact live head)
Risk: Moderate — ACP edit handling crosses admission, session identity, queue cancellation, reactions, prompt/reply routing, setup mode, and terminal recovery.
Findings
No unresolved author-actionable defects.
The refreshed head resolves the two defects reported on the prior head:
- Interrupted edit work is retained as cancelled prior context and merged into the replacement turn with supersede framing; it is not re-run ahead of the newer request.
- When the original cannot be verified after bounded lookup, setup nudges and terminal notices no longer assert the edit target as a thread root. They fall back to channel top level rather than publishing ancestry the relay can reject. A later terminal-notice lookup can still recover the verified original route.
Independent systems/integration and product/adversarial passes also traced the resolved route through same-channel and effective-author admission, private membership checks, SessionScope, batching/requeue, worker affinity, prompt/context/reply anchors, reactions, setup nudges, terminal failure retry, native steering, and channel removal. Resolved top-level and threaded originals consistently use the visible original; unresolved originals degrade without inventing ancestry. No visual or accessibility surface changed.
Author action: none.
Validation
- Live PR head rechecked immediately before review:
e079ba645646ddcb7fe35cb74b6c4418a0718132. - Authenticated reviewer:
jedwards27; live author:loganj. git diff --check 0ee609379004a894e99b449885c37a044f41919e HEADpassed at detached clean heade079ba645646ddcb7fe35cb74b6c4418a0718132before checkout cleanup.- Focused source/regression audit covered threaded, top-level, and unresolved originals; signed notice/nudge placement; reaction targets; session scope; and interrupted-edit supersession.
- Current-head required CI is green, including Rust unit tests/lint, macOS and Windows builds, Desktop smoke/integration, relay integration, PostgreSQL, security, Semgrep, zizmor, and DCO.
Confidence gaps and residual risk
All three local Rust-suite attempts were blocked before compilation by this reviewer host's unaccepted Xcode license (cc exit 69). This is a reviewer-tooling gap, not author rework. The prior live-relay run at e5d3c4f2 established top-level/threaded routing and exposed the two defects fixed by this refreshed head; the refreshed-head evidence is source tracing, focused regressions, and CI rather than another live relay run. The separately cancelled Codex security-review job is non-required and does not override the passing required security gates.
Verification owner: CI and a reviewer environment with an accepted Xcode toolchain for any additional local rerun. Any new head invalidates this approval pending delta review.
|
Re the Codex security review finding ("Default subscriptions never deliver message edits"): this is the planned boundary of the stack, not a defect in this PR. #4741 only sets where replies to edits go and is inert by default, as the description says. #6131 (head 70f4d4b, stacked on this PR's head e079ba6) adds |
…ty-from-device * origin/main: fix(acp): preserve routing for edited messages (#4741) fix(mobile): distinguish unknown presence on identity surfaces (#7383) fix(mobile): hydrate and refresh relay presence snapshots (#7382) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
Main added four commits since the last merge: the desktop Admin Console Actions tab (#7904), two mobile presence fixes (#7382, #7383) and ACP routing for edited messages (#4741). They change buzz-acp, desktop and mobile only. No file is changed on both sides, and main adds no migration, so the private tables stay at 0056. The merge is textually clean and needs no follow-on edit. buzz-db, buzz-relay, migrations, schema and Cargo.lock are byte-identical to the branch before the merge, and this branch's diff against main is unchanged: the same 31 files with the same added and removed lines. Co-authored-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Signed-off-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
This branch was successfully deployed
4 participants
🤖
Summary
When someone edits a message to add an @mention, the app does not change the original message. It publishes a separate hidden "edit" event that points back at it. An agent that handles the edit must work on the original message: it reacts, replies, and posts any error notice on that message or in its thread. Before this PR, an agent treated the hidden edit as a new message. It replied in the wrong place, and with per-thread sessions it started a new, empty conversation.
This is the first of three PRs. It sets where agents reply to edits. It does not yet make edits wake agents by default (#6131 does that).
threadsession policy, where each thread has its own agent conversation): the edit joins the original message's thread session. It does not start a new session.Details
main.Stack
1 of 3, based on
main. Next: #6131 (edits wake agents by default), then #6132 (edits join a turn that is already running).Related issue
Part of #2540. The issue is fixed when #6131 merges.
Testing
cargo test -p buzz-acp --all-targetspasses. One existing test,session_new_forwards_complete_git_block_without_duplicate_names, fails in a shell where the git settingnostr.keyfileis set. It passes when the setting is removed. This PR does not touch that test.