Fix race condition causing requested commands to be auto-cancelled.#10241
Fix race condition causing requested commands to be auto-cancelled.#10241
Conversation
szgupta
force-pushed
the
suraj/fix-active-convo-id-race
branch
from
86b1197 to
46a26eb
Compare
szgupta
force-pushed
the
suraj/fix-active-convo-id-race
branch
from
46a26eb to
e64a356
Compare
szgupta
changed the title
szgupta
changed the title
|
@zachbai this is a short-term fix. Real fix is consolidating these terminal view <> convo ID maps and their access patterns. It's a big mess right now and will require a good amount of work to avoid regressing stuff. So this is an in-between to get the halting problem fixed. |
We should be able to get rid of the |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR separates explicit conversation transfer from automatic active-conversation marking and adds a fallback lookup for requested command actions.
Concerns
- The global fallback in
conversation_id_for_actiondrops the terminal-view ownership constraint and can associate a requested command with a conversation from another pane.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| // Fallback: the conversation may have been transferred to another view | ||
| // or removed from the live list while the action was executing. Search | ||
| // all in-memory conversations so the command is not silently dropped. | ||
| let fallback = self |
There was a problem hiding this comment.
TerminalView then executes the requested command in the current pane's active session. Keep the fallback scoped to conversations owned by terminal_view_id (or use an action→conversation mapping) rather than searching every conversation.
2 participants
Description
When two agent conversations run in parallel in different terminal views,
set_active_conversation_idwas called during automatic follow-ups (tool call → result cycles). This function has transfer semantics: it removes the conversation from every other terminal view's live list. When two views both trigger follow-ups concurrently, each view's follow-up could steal the other view's conversation, causing requested commands to be silently dropped with:Root cause
set_active_conversation_idwas designed for explicit user navigation (opening a conversation in a different pane from the command palette), but was also being called in two automatic code paths:send_follow_up_for_conversation(controller.rs) — after tool call results are sent backsend_request_input(controller.rs) — when sending any non-passive requestThese paths only need to update the "most recently streamed" pointer within their own view. They should never touch other views.
Fix
history_model.rs: Addedmark_active_conversation_id— a non-transferring variant that updates the active conversation pointer for a single terminal view without removing the conversation from other views' live lists. The existingset_active_conversation_id(with transfer semantics) is preserved for explicit navigation call sites.controller.rs: Switched both follow-up call sites to usemark_active_conversation_idinstead ofset_active_conversation_id.history_model.rs(conversation_id_for_action): Added a global fallback search across all in-memory conversations when the per-view lookup fails. This is a safety net — if a conversation is ever removed from a view's live list while actions are in flight, the command still executes instead of being silently dropped.Linked Issue
N/A — discovered via internal debugging of intermittent command cancellation during parallel agent conversations.
Testing
Manual testing with parallel agent conversations in split panes. The race condition is timing-dependent and not consistently reproducible, but the fix is structurally sound: the follow-up paths no longer call the transferring variant, so the race is eliminated by design.
Agent Mode
Co-Authored-By: Oz [email protected]
Conversation