Fix run command executor race condition#10666
Conversation
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![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 reorders legacy SSH remote-server bootstrapping so the daemon receives SessionBootstrapped before local session initialization emits subscriber events that can enqueue RunCommand requests. The changed ordering appears consistent with the remote-server client queueing model and I did not find a concrete security issue in the diff.
Concerns
- Manual testing is required for changes that can be manually tested. This is a user-perceivable behavior fix, but the PR description leaves local manual testing unchecked and includes no screenshot, recording, or justification for why manual testing evidence is not possible.
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
## Description We were calling `initialize_bootstrapped_session` (which downstream calls command executor before sending the notification to the server). This happens in the case when shell bootstrap happens before the server establishes connection This ordering could cause problems because we have not sent the session bootstrapped notification to the server yet. Hence the following run command execution within initialize_bootstrapped_session will fail on the server with this sentry error: https://warpdotdev.sentry.io/issues/7468335682/events/ded45b2a39c64373900a13f84095a97f/
## Description We were calling `initialize_bootstrapped_session` (which downstream calls command executor before sending the notification to the server). This happens in the case when shell bootstrap happens before the server establishes connection This ordering could cause problems because we have not sent the session bootstrapped notification to the server yet. Hence the following run command execution within initialize_bootstrapped_session will fail on the server with this sentry error: https://warpdotdev.sentry.io/issues/7468335682/events/ded45b2a39c64373900a13f84095a97f/
2 participants
Description
We were calling
initialize_bootstrapped_session(which downstream calls command executor before sending the notification to the server). This happens in the case when shell bootstrap happens before the server establishes connectionThis ordering could cause problems because we have not sent the session bootstrapped notification to the server yet. Hence the following run command execution within initialize_bootstrapped_session will fail on the server with this sentry error: https://warpdotdev.sentry.io/issues/7468335682/events/ded45b2a39c64373900a13f84095a97f/