Couple LongRunningCommandAgentInteractionState with block id#27
Conversation
|
|
||
| /// How the agent is interacting with the current long running command (if at all). | ||
| /// Deprecated in favor of long_running_command_agent_interaction. | ||
| pub long_running_command_agent_interaction_state: |
There was a problem hiding this comment.
should also skip serializing if this is None, no?
There was a problem hiding this comment.
Yea might as well add it now. Technically just need #[serde(default)] once the field is no longer populated
|
|
||
| /// How the agent is interacting with a specific long running command block. | ||
| #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] | ||
| pub struct LongRunningCommandAgentInteraction { |
There was a problem hiding this comment.
Rather than adding a separate type and having to deal with migrating from one LRC type to another, I actually think it could be ok to just add active_block_id as a sibling field? Is that enough to determine if the update should be applied (wouldn't block_id as populated by the sharer always be the active block ID)
There was a problem hiding this comment.
then you don't have to deal with a semantic either/or migration for the new field and old field, can just say there is a new field only supported by newer clients?
There was a problem hiding this comment.
The complication with that is that UniversalDeveloperInputContext is a merging of several sources of writes to a shared cache. If we put active block id on this, it would either need to be populated on every source (and probably all of these updates should be guarded on matching block id), or we say it's specifically for the LRC update which is weirder imo.
I think coupling to LRC update is what I want. For this update we need the block id to come from the source of updating LRC state, not the active block id at time of sending UniversalDeveloperInputContext
## Description Was investigating sources of ambient agent tasks becoming canceled by user automatically: https://linear.app/warpdotdev/issue/REMOTE-1887/maa-requests-getting-canceled#comment-75459dec It can happen when we receive a NotInteracting LRC state, which is interpreted as user takeover of a CLI subagent monitored requested command. This is meant to cancel the conversation stream because the agent is no longer monitoring, but it also set the conversation status to canceled, which updates the ambient agent state to canceled by user. But I also see the sharer receiving NotInteracting from the viewer when not expected. I believe there may be stale updates from the viewer causing this. We definitely should have LRC state updates apply to a specific block so we can discard stale updates. And finally, some bugs with cloud mode were causing echoes of LRC state updates. The problematic flow was: - Sharer updates LRC state, fans out result - Cloud mode viewer applies LRC update from sharer locally - But cloud mode viewer mutation locally would result in LRC update, which gets fanned out to sharer because cloud mode viewer has write permission - Sharer receives echo of its LRC update, as if viewer wrote it So this is a combination of 3 fixes: - User takeover of a LRC should keep the conversation and task status in progress - In shared sessions, LRC state updates should apply to specific blocks so stale updates can't apply - Prevent viewer replaying sharer LRC updates from writing again to shared session Requires protocol change to be merged first, before updating cargo.toml: warpdotdev/session-sharing-protocol#27 ## Testing <!-- How did you test this change? What automated tests did you add? If you didn't add any new tests, what's your justification for not adding any? Manual testing is required for changes that can be manually tested, and almost all changes can be manually tested. If your change can be manually tested, please include screenshots or a screen recording that show it working end to end. You can run the app locally using `./script/run` - see WARP.md for more details on how to get set up. --> - [x] I have manually tested my changes locally with `./script/run` ### Screenshots / Videos <!-- Attach screenshots or a short video demonstrating the change, where appropriate. Remove this section if it is not relevant to your PR. --> Tested with warpdotdev/session-sharing-protocol#27 https://www.loom.com/share/f09e7b1d7e35412a99a7c2947104a460
2 participants
See warpdotdev/warp#12384