Skip to content

devcontainer: Fix OpenDevContainer action panic due to double workspace entity lease#49058

Merged
KyleBarton merged 3 commits intozed-industries:mainfrom
oliverbarnes:fix-devcontainer-model-panic
Feb 13, 2026
Merged

devcontainer: Fix OpenDevContainer action panic due to double workspace entity lease#49058
KyleBarton merged 3 commits intozed-industries:mainfrom
oliverbarnes:fix-devcontainer-model-panic

Conversation

@oliverbarnes
Copy link
Contributor

@oliverbarnes oliverbarnes commented Feb 12, 2026

Closes #49055

Heads up: This might be a naïve solution. I ran into the issue after merging latest main into #48896, and confirming that it was unrelated to that PR and incoming from upstream.

Agent one-shot the fix, it works and tests pass. But I'm still wrapping my head around the changes that led to the bug. I figured the breakage is bad enough (I couldn't open devcontainers at all) to submit a possibly naïve fix.

Fix

Hoists the find_devcontainer_configs call out of new_dev_container and into the call site, where we already have a direct &mut Workspace reference that doesn't go through the entity map. The computed configs are passed into new_dev_container as an argument.

What was happening

After #48800 ("Re-add MultiWorkspace"), with_active_or_new_workspace nests a Workspace entity lease inside a MultiWorkspace entity lease. The OpenDevContainer handler was also changed from async to sync in the same PR, so RemoteServerProjects::new_dev_container now runs while Workspace is leased. Inside new_dev_container, a WeakEntity<Workspace>::read_with call tries to read Workspace through the entity map, finds it already leased, and panics.

Release Notes:

  • Fixed a panic when opening the dev container modal via the OpenDevContainer action.

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Feb 12, 2026
@yeskunall
Copy link
Member

If you build your changes and try your reproduction steps @oliverbarnes I think this still crashes Zed. Can you confirm?

@yeskunall
Copy link
Member

@oliverbarnes this still crashes Zed.

@oliverbarnes
Copy link
Contributor Author

oliverbarnes commented Feb 12, 2026

@yeskunall it's working for me. Tried again just now - opening a devcontainer either from the command palette or from the modal (toast?) widget that pops up, both work

@yeskunall
Copy link
Member

@oliverbarnes not trying to be argumentative, but it does not. See recording (apologies for the quality -- having issues recording today).

Built-in.Retina.Display.2026-02-12.15.06.19.mp4

@oliverbarnes
Copy link
Contributor Author

@yeskunall all good :) had the same idea and was recording over here. The difference is that I'm passing a project in the command line. Will try your scenario as well. Next time we could just hop on a call and share our screens :)

Screen.Recording.2026-02-12.at.20.13.53.720p.mov

@yeskunall
Copy link
Member

yeskunall commented Feb 12, 2026

The difference is that I'm passing a project in the command line.

@oliverbarnes ah, that makes sense. If you take a look at the code that launches the modal for the Dev Containers, you’ll see there is another call site that causes a panic. I was actually making a PR to fix this, when I noticed yours.

Next time we could just hop on a call and share our screens :)

@/KyleBarton is our Dev Containers SME, which is why you’ll see Mikayla assigned it to them. That being said -- happy to. My calendar is public and open to anyone. Book any available slot :)

@oliverbarnes
Copy link
Contributor Author

Got it :) But so, I'm not sure how to repro this other panic... I just ran cargo run from Zed project root, and the devcontainer did spin up. BUT, and this happens intermittently, sometimes the devcontainer spins up right away, without my confirming, and sometimes I get the modal:

Screen.Recording.2026-02-12.at.20.24.16.720p.mov

@oliverbarnes
Copy link
Contributor Author

Your call - if you have a fix that handles all scenarios, by all means ship it! Happy to close this if that's the case.

@yeskunall
Copy link
Member

yeskunall commented Feb 12, 2026

Happy to help here! There is still a difference in how I’m testing, vs. how you are in your latest recording. Notice in mine, I do not accept the prompt to open the project in a Dev Container, and instead directly try to open it using the action.

Your call - if you have a fix that handles all scenarios, by all means ship it! Happy to close this if that's the case.

If you can also reproduce the crash using my steps, that’d be great!

@oliverbarnes
Copy link
Contributor Author

OOoh, when I try to open from the command palette after canceling the initial build it does crash for me here. Gonna take this into account and iterate, then. Stay tuned

@oliverbarnes
Copy link
Contributor Author

oliverbarnes commented Feb 12, 2026

@yeskunall tested all the scenarios we discussed here, all working for me now.

I wish there was a way to cover all these with tests...

@yeskunall
Copy link
Member

@oliverbarnes this still applies:

@/KyleBarton is our Dev Containers SME, which is why you’ll see Mikayla assigned it to them.

I’ll let Kyle confirm if the changes are sound, but you at least did find the call site where the panic happens.

I wish there was a way to cover all these with tests...

Yup, I agree with you. I’m from the front-end (web) world where we’d cover these outside of unit tests, but not sure how you’d write e2e/integration test(s) for Zed. Haven’t even given it a single thought.

@oliverbarnes
Copy link
Contributor Author

oliverbarnes commented Feb 12, 2026

Yeah, no idea how E2E tests would work here, with external dependencies on the CLI, Docker... I guess it's doable, but super heavy duty.

Claude suggested adding a couple of tests at the action dispatch level, following an existing testing pattern. Nice and lightweight. They manage to reproduce the panic when ran without the fix, and they pass with it, so I've gone ahead and pushed them.

@KyleBarton
Copy link
Collaborator

@oliverbarnes thanks for this - it looks right to me. @yeskunall can you just confirm that the new changes work for you as well, since you saw an earlier crash?

I think there's probably additional work to make multi-workspace devcontainer selection nicer, but that should probably be in a future change.

@yeskunall
Copy link
Member

@KyleBarton I just pulled the changes and rebuilt Zed and it all looks good on my end now. Thank you both!

@KyleBarton KyleBarton merged commit 1a49170 into zed-industries:main Feb 13, 2026
41 of 43 checks passed
@oliverbarnes oliverbarnes deleted the fix-devcontainer-model-panic branch February 13, 2026 08:57
@oliverbarnes
Copy link
Contributor Author

Yup, I agree with you. I’m from the front-end (web) world where we’d cover these outside of unit tests, but not sure how you’d write e2e/integration test(s) for Zed. Haven’t even given it a single thought.

@yeskunall coincidentally this came up on my feed today: https://simonwillison.net/2026/Feb/7/software-factory/

He writes about an experiment using what they call a Digital Twin Universe - a complete E2E/QA environment running mock clones of all the external services their software needs to integrate with (Okta, Slack, Jira, several Google services). All written by agents.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenDevContainer action panics with "cannot read workspace::Workspace while it is already being updated"

3 participants