feat(acp): add conversation-scoped session experiment#1981
Open
baxen wants to merge 11 commits into
Open
Conversation
Co-authored-by: npub1qvn3cujt28pg06ehlstrxyz6ayzp06t4uc7r566vxwwgrv24hglq9zju0n <03271c724b51c287eb37fc1633105ae90417e975e63c3a6b4c339c81b155ba3e@sprout-oss.stage.blox.sqprod.co> Co-authored-by: npub1k766u8wq088nkr9vy94uqlh9kmw5956g593a7xk7cks7frhgctmsr00p27 <b7b5ae1dc079cf3b0cac216bc07ee5b6dd42d348a163df1adec5a1e48ee8c2f7@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1qvn3cujt28pg06ehlstrxyz6ayzp06t4uc7r566vxwwgrv24hglq9zju0n <03271c724b51c287eb37fc1633105ae90417e975e63c3a6b4c339c81b155ba3e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1k766u8wq088nkr9vy94uqlh9kmw5956g593a7xk7cks7frhgctmsr00p27 <b7b5ae1dc079cf3b0cac216bc07ee5b6dd42d348a163df1adec5a1e48ee8c2f7@sprout-oss.stage.blox.sqprod.co>
…surfaces Reduce the ongoing cost of the top-level-sessions experiment by collapsing its scattered maintenance surfaces without changing behavior: - pool: replace eager session_owners reconciliation (5 lifecycle call sites + retains in return_agent / invalidate_channel_sessions / model-switch paths) with a single lazy prune_session_owners() at the one read site in try_claim. Future slot-lifecycle paths can no longer forget the purge; the invariant converges at one choke point. - queue: extract pop_cancelled() / dispatch_cancelled() helpers so the three copy-pasted cancelled-batch blocks in flush_next become one-liners. - session key: drop the top_level_sessions bool param and dead thread-tag fallback from conversation_session_key; remove the redundant PromptContext.top_level_sessions field. The key is now purely data-driven from batch.conversation_root, which is only populated at queue-push when the experiment is enabled. Net -59 lines. cargo test -p buzz-acp: 530 passed; clippy and fmt clean. Disabled-mode paths degenerate identically (root=None everywhere). Co-authored-by: npub1zurdm6fx3kksz8f8d8y4js3mvjyu3rshlpqejr8wa8hta5tsqqeshmes7w <1706dde9268dad011d2769c959423b6489c88e17f841990ceee9eebed1700033@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1zurdm6fx3kksz8f8d8y4js3mvjyu3rshlpqejr8wa8hta5tsqqeshmes7w <1706dde9268dad011d2769c959423b6489c88e17f841990ceee9eebed1700033@sprout-oss.stage.blox.sqprod.co>
baxen
force-pushed
the
mini/acp-top-level-sessions
branch
from
July 18, 2026 19:05
bc59914 to
f442707
Compare
baxen
marked this pull request as ready for review
July 18, 2026 19:05
![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: f442707444
ℹ️ 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".
Comment on lines
+473
to
+474
| #[arg(long, env = "BUZZ_ACP_TOP_LEVEL_SESSIONS", default_value_t = false)] | ||
| pub top_level_sessions: bool, |
There was a problem hiding this comment.
When this new flag is enabled directly via buzz-acp --top-level-sessions / BUZZ_ACP_TOP_LEVEL_SESSIONS (outside Desktop's spawn finalizer), the harness still defaults --multiple-event-handling to steer, and the mid-turn gate/signaling remains channel-scoped (mode_gate_signal / signal_in_flight_task). In that setup, a new top-level message in the same channel while another root is in flight can be steered or cancel-merged into the active root's ACP session instead of waiting for its own isolated session, defeating the advertised root isolation; force or reject non-queue handling when this flag is set.
Useful? React with 👍 / 👎.
Comment on lines
+1533
to
1534
| let is_new_channel_session = !agent.state.contains_session(session_key); | ||
| if is_new_channel_session && !agent.state.canvas_sections.contains_key(cid) { |
There was a problem hiding this comment.
With top-level sessions enabled, is_new_channel_session becomes true for each fresh conversation root, but this channel-scoped canvas_sections guard skips fetching whenever any older root session in the channel populated the cache. If the channel canvas changes between two top-level conversations, the new ACP session is created with the stale canvas metadata until every retained root session for that channel is invalidated or evicted; key the canvas cache by the session key or refetch on each new root session.
Useful? React with 👍 / 👎.
Three fixes for PR CI: - fmt: apply rustfmt to the rebased command_env helper in managed_agents/runtime/tests.rs (Rust Lint failure). - lib.rs over the 1000-line limit: move the window/haptic tauri commands (perform_sidebar_default_haptic, title_bar_double_click, fill_window, toggle_maximize) into commands/window_actions.rs where they belong with the other frontend-forwarded commands (1012 -> 901 lines, back under the default limit). - app_state.rs over its ratchet: drop the AppState field for the experiment entirely. The experiment flag now lives in commands/experiments.rs as a process-local atomic, lazily hydrated from the Rust-owned store on first read. The hydrate-before-restore ordering invariant now holds by construction (the spawn boundary is the first reader) instead of by an app-setup ordering requirement, and app_state.rs is byte-identical to main — one less shared surface the experiment touches. desktop src-tauri: cargo test 1483 passed, clippy and fmt clean, and node scripts/check-file-sizes.mjs passes. Co-authored-by: npub1zurdm6fx3kksz8f8d8y4js3mvjyu3rshlpqejr8wa8hta5tsqqeshmes7w <1706dde9268dad011d2769c959423b6489c88e17f841990ceee9eebed1700033@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1zurdm6fx3kksz8f8d8y4js3mvjyu3rshlpqejr8wa8hta5tsqqeshmes7w <1706dde9268dad011d2769c959423b6489c88e17f841990ceee9eebed1700033@sprout-oss.stage.blox.sqprod.co>
Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co>
…affinity, control resolution
Scheduling identity becomes ConversationSessionKey (channel_id, root_event_id):
- queue.rs: all in-flight control state (deadlines, batch sizes, retry,
cancel sets, withheld native steer) keyed by scope; flush_next selects
the oldest non-in-flight scope so distinct roots flush concurrently;
restore_unclaimed is the exact lossless undo of flush_next.
- pool.rs: TaskMeta carries the scope; try_claim returns
ClaimOutcome{Claimed,BusyOwner,Exhausted} with strict retained-root
slot affinity and lazy owner pruning; send_steer scope-keyed.
- lib.rs: dispatch_pending dispatches multiple scopes per pass, holding
BusyOwner/Exhausted batches until after the loop (prevents
flush→restore→reflush livelock) then restoring losslessly; control
resolution never fans out across roots — explicit root targets
exactly, channel-level control resolves only when exactly one scope
is in flight (Err(n) = ambiguous, touch nothing); steer fork is
scope-exact. Channel mode remains the degenerate root=None path.
Deterministic coverage: cross-root concurrent dispatch, busy-owner skip
without root migration, pool-exhaustion lossless restore, three
restore_unclaimed exactness tests, control-scope resolution (channel
exactly-one rule, explicit control-frame root, threaded-event root
targeting, channel-mode fallback). cargo test -p buzz-acp: 557 passed.
Co-authored-by: Tyler Longwell <tlongwell@block.xyz>
Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co>
…cope Reviewer-authored deterministic coverage for the pool-affinity edges Eva flagged unwritten at eca05aa (LRU-eviction-under-load, worker death/respawn). All test-only, in the pool.rs test module — no production changes. - quinn_lru_evict_stale_reservation_does_not_beat_real_session_holder: a stale session_owners[key]->slot reservation must not win over a different slot that actually holds the session. Proven non-vacuous: fails (claims stale slot 0 vs correct slot 1) when prune_session_owners is neutered, so prune is load-bearing for affinity correctness (prevents a forked provider session). - quinn_bare_stale_reservation_is_pruned: bare stale reservation is dropped by prune -> clean Pass 2 claim, no phantom session. - quinn_busy_owner_does_not_block_unrelated_root: a busy retained root returns ClaimOutcome::BusyOwner (never migrates, never Exhausted) while an unrelated root claims another idle slot concurrently. - quinn_respawn_clears_stale_reservation: a respawned fresh-state agent at the same index does not inherit the dead session via a stale reservation. cargo test -p buzz-acp: 561 passed. clippy -D warnings clean, fmt clean. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co>
Two review findings from the PR #1981 session-scope setting: Legacy materialization (Mari, MAJOR 1): hydration read the legacy acp_top_level_sessions override but never persisted the translation, so removing the compatibility reader would silently flip explicit channel users to the thread default. hydrate_scope now atomically materializes an explicit legacy value into acp_session_scope during hydration — false→channel, true→thread — while the bare no-field default stays unwritten (unset remains distinguishable from an override) and an explicit new-field value is never rewritten. DesktopSettings gains a #[serde(flatten)] passthrough so any save preserves JSON fields owned by other features. A failed materialization write keeps the translated scope in memory and the legacy field on disk for retry — no divergence. Rollback convergence (Mari, MAJOR 2): on apply failure with a failed rollback write, the old path restarted processes and forced the UI to 'previous' while the authoritative backend still held the new value — false convergence. applyAcpSessionScopeSetting now establishes the authoritative scope first (confirmed rollback write, else re-read via getBackend), reconciles processes and UI to that actual value, and if the authority is unreadable fires onUnrecoverable — the card surfaces a hard recovery state and disables the toggle instead of claiming a scope. Tests: 7 new Tauri hydration/materialization tests (incl. read-only-dir write-failure injection) and 2 new rollback-matrix tests (authority re-read reconciliation; double-failure hard recovery). Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
Mari's re-review: after a rollback/reconciliation pass, a failed stopAgent leaves a process running under the wrong scope and a failed startAgent leaves it stopped — committing a normal UI scope in either case claims a convergence that does not exist. All process rollbacks are still attempted, but any failure now surfaces onUnrecoverable (card disables the toggle) instead of setUi. Tests: the one-rollback-restart-fails case now asserts hard recovery and no UI claim; new rollback-stop-failure case asserts the same. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
DMs are private channels, not a separate scoping model. Pin that contract with four tests in dispatch_scope_tests: - scope derivation is channel-type-agnostic: identical tag shapes derive identical root components on DM and stream channels in both modes - thread mode: two top-level DM messages are distinct concurrent roots on distinct workers (context registers the channel as a DM) - thread mode: a threaded DM reply joins its outermost root's batch - channel mode: DM events collapse to (channel, None) and serialize, second worker idle Test-only; no production code touched. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.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
1 participant
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
Long-lived channel-keyed ACP sessions accumulate unrelated conversations and can consume provider context windows quickly. This experiment isolates top-level conversations while preserving reply continuity.
What
Risk Assessment
Medium — this touches ACP queueing, session affinity, and managed-agent startup, but the new behavior is disabled by default and covered by focused Rust, Tauri, and frontend tests. Remote agents are unchanged.
References
cargo test -p buzz-acp— 530 passedcargo clippy -p buzz-acp --all-targets -- -D warnings— passedGenerated with Codex