Skip to content

dev_container: Align compose project name with reference CLI - #54302

Merged
KyleBarton merged 23 commits into
zed-industries:mainfrom
antont:fix-compose-project-name-derivation
Apr 22, 2026
Merged

KyleBarton merged 23 commits into
zed-industries:mainfrom
antont:fix-compose-project-name-derivation

Conversation

@antont

@antont antont commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Draft / open question for maintainers. The failure mode this fixes is narrow — a new-Zed-created container exists under the name-field project while a CLI-derivation tool (@devcontainers/cli, VS Code) operates on the same folder (the container persists in Docker, so the originating Zed session doesn't need to still be open). See issue #54255 failure mode 3 and the fixture's step 6.

I'd like to pose this as a question rather than a claim: is matching @devcontainers/cli's getProjectName precedence something the project wants to take on, given the narrowness of the bug? I wrote this implementation mostly as a way to explore what parity would actually cost — happy to close it if you'd rather leave it as-is, or pare it down (e.g. just rule 4) if a partial match is preferable.

The broader value beyond this specific bug: devcontainer impls agreeing on the same project name means containers created by Zed, the devcontainer CLI, and VS Code are interchangeable for the same folder, which feels worth it to me — but you know the project's priorities better.

Folds in #54068 (detection) — closing that PR unmerged; its MultipleMatchingContainers error lands here.

Self-Review Checklist:

  • I've reviewed my own diff for quality, security, and reliability
  • Unsafe blocks (if any) have justifying comments
  • The content is consistent with the UI/UX checklist
  • Tests cover the new/changed behavior
  • Performance impact has been considered and is acceptable

Closes #54255

Summary

Match @devcontainers/cli's full getProjectName precedence. Replaces safe_id_lower(devcontainer.json's name) with the five-step chain the reference CLI walks (see src/spec-node/dockerCompose.ts in devcontainers/cli):

  1. COMPOSE_PROJECT_NAME from the local environment.
  2. COMPOSE_PROJECT_NAME= in the workspace .env file.
  3. Top-level name: on the merged compose config, when at least one fragment declared it explicitly.
  4. ${workspaceFolderBasename}_devcontainer — only when the first compose file's directory is <workspace>/.devcontainer/.
  5. Otherwise, the plain basename of the first compose file's directory (no suffix).

The old Zed implementation diverged at every one of those inputs: any user setting COMPOSE_PROJECT_NAME, shipping a .env with one, declaring a top-level compose name:, or pointing dockerComposeFile outside .devcontainer/ (e.g. "../docker-compose.yml") got a different project namespace than the CLI and VS Code, producing two compose projects for the same folder.

Adds a small sanitize_compose_project_name() helper implementing the CLI's rules (lowercase + strip [^-_a-z0-9]) — notably preserving hyphens, which safe_id_lower would have replaced with underscores.

Adds two helpers used by the precedence walk:

  • parse_dotenv_compose_project_name — line scan extracting COMPOSE_PROJECT_NAME=… from the workspace .env, matching the subset the CLI's regex dotenv reader recognizes.
  • compose_fragment_declares_name — parses each compose fragment with yaml-rust2 (already a transitive workspace dep; slated to become a direct dep via Dev Containers don't support podman-compose #53922) and checks for a name key on the root mapping (block, quoted, or flow style all work), matching the CLI's own yaml.load. docker compose config always injects name: devcontainer into its merged output when no fragment declared one, so rule 3 needs to distinguish the user-provided case from the injected default — this helper supplies that signal. On YAML parse failure it returns "not declared" (rule 4 applies), matching the CLI's fallback.

project_name() becomes async and fallible (async fn project_name(&self) -> Result<String, DevContainerError>) so it can load the .env file and each compose fragment via self.fs.load. Four call sites now .await? the derivation. Real I/O errors on the .env read propagate as FilesystemError (matching the CLI's narrow ENOENT/EISDIR swallow); fragment-rescan read errors are logged and skipped (matching the CLI's broader try/catch over its fragment read + parse).

The name field is still used as the features image-tag prefix (generate_features_image_tag); only the compose project namespace is decoupled from it.

Duplicate-container detection (from #54068). When check_for_existing_container's label-based lookup returns more than one match, propagate MultipleMatchingContainers(ids) with instructions to clean up the stale one(s). This covers the mixed-version upgrade edge case where a pre-fix Zed left a container under the legacy project name alongside a CLI-style one — transparent to users in the common case (one tool, one container), explicit error when two legacy siblings need manual cleanup.

Why

Full write-up with verified fixtures and captured output: #54255.

Three failure modes from the same root cause, all resolved by this change:

  1. Interop — opening a folder in both Zed and devcontainer up (or Zed and VS Code) creates two compose projects with identical devcontainer.local_folder + devcontainer.config_file labels, breaking the spec's uniqueness invariant.
  2. Cross-worktree silent db/volume reuse — if multiple git worktrees share a devcontainer.json with the same name, Zed uses the same compose project for all of them; Compose reuses stateful siblings (db, cache, localstack) by config-hash, so worktree B silently inherits worktree A's database. Fixture + captured output: antont/zed-devcontainer-db-share-repro.
  3. Mixed-version Zed sessions — the Rust impl landed in stable v0.232.2 (2026-04-15, Dev containers native implementation #52338). Older Zed (≤v0.231.x) shelled out to @devcontainers/cli so it used the reference derivation. The collision shows up when a new-Zed-created container exists under the name-field project while a CLI-derivation tool (old Zed, devcontainer up, VS Code) operates on the same folder.

Migration / compatibility

Existing Zed-created containers (under the old safe_id_lower(name) project) continue to be found via check_for_existing_container's label-based lookup — they're looked up by devcontainer.local_folder + devcontainer.config_file, not by project name. A user with duplicate legacy containers from a prior Zed session sees MultipleMatchingContainers with cleanup instructions.

Revision — 2026-04-22

Revised per @KyleBarton review on the prior version:

Test plan

  • cargo test -p dev_container --lib — 89 passed, including:
    • sanitize_compose_project_name_matches_cli_rules
    • --project-name assertion added to test_spawns_devcontainer_with_docker_compose
    • check_for_existing_container_errors_when_multiple_match
    • derive_project_name_env_wins_over_everything
    • derive_project_name_dotenv_wins_over_compose_and_fallback
    • derive_project_name_compose_name_wins_over_fallback
    • derive_project_name_skips_compose_name_when_not_explicitly_declared
    • derive_project_name_omits_suffix_when_compose_file_outside_devcontainer_dir
    • derive_project_name_normalizes_compose_path_for_rule_4
    • compose_fragment_declares_name_detects_top_level_name_key (covers block, quoted-key, and flow-style roots, plus parse failure → not-declared)
    • is_missing_file_error_only_accepts_notfound_and_isadirectory
  • cargo fmt --all — clean
  • ./script/clippy -p dev_container — clean
  • End-to-end with fixture antont/zed-devcontainer-compose-test:
    • Build zed from this branch.
    • Clean slate: docker ps -a --filter "label=devcontainer.local_folder=$PWD" -q | xargs -r docker rm -f
    • zed --dev-container /path/to/devcontainer-compose-test → Zed creates container under project devcontainer-compose-test_devcontainer (was compose_duplicate_repro before the fix).
    • devcontainer up --workspace-folder $PWD → CLI reports the same containerId Zed created; no second compose project is introduced.
    • Captured: devcontainer-compose-test_devcontainer-app-1, composeProjectName: "devcontainer-compose-test_devcontainer" reported by both tools.

Release Notes:

  • Fixed dev container Docker Compose project name now matches the full getProjectName precedence from the reference devcontainer CLI (COMPOSE_PROJECT_NAME in the environment, then in the workspace .env, then an explicit top-level name: on the merged compose config, then the basename of the first compose file's directory — with the _devcontainer suffix only when that directory is <workspace>/.devcontainer). This prevents duplicate containers when the same folder is opened with both Zed and the devcontainer CLI / VS Code.

@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Apr 20, 2026
@antont
antont marked this pull request as ready for review April 20, 2026 09:11
@KyleBarton
KyleBarton self-requested a review April 21, 2026 21:39

@KyleBarton KyleBarton left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the deep diving here! Some feedback:

  • based on my comment in devcontainer_manifest, do we need do be parsing yaml here, or can we rely on the json representation of the docker compose manifest in order to decide whether a name is explicitly specified? And if this isn't needed, can we remove the yaml parsing dependency?
  • If we do need to pull in a yaml dependency here, then we should probably pull in rust-yaml2, since it is a transitive dependency in Zed already. This is discussed a bit here: #53922 (the work of which will requires us to eventually take this dependency, I think)
  • I'm content to close #54068 and just focus on getting this more holistic change over the line -- any objections?

Comment thread crates/dev_container/src/devcontainer_manifest.rs
Comment thread crates/dev_container/src/devcontainer_manifest.rs Outdated
antont and others added 18 commits April 22, 2026 17:51
…ar error

Per Dev Containers spec, the identifying labels
(devcontainer.local_folder + devcontainer.config_file) should be
unique per project. When two tools (e.g. Zed + the reference
devcontainer CLI) derive different compose project names from the
same folder, both containers end up carrying these labels and
`docker ps` returns more than one match.

Previously the generic `evaluate_json_command` helper crashed on the
resulting newline-delimited JSON. Silently picking the first value
would hide the duplicate state and could connect Zed to the wrong
container. Instead, keep the generic helper strict (one JSON value
per call) and move NDJSON awareness into `find_process_by_filters`:

- 0 matches -> Ok(None)
- 1 match   -> Ok(Some(..))
- >=2       -> Err(MultipleMatchingContainers(ids)) with a Display
               message that names the duplicate IDs and describes
               how to resolve.

The new error variant is passed through unchanged in
`start_dev_container_with_config` so its crafted Display reaches the
UI prompt instead of being swallowed by `DevContainerUpFailed`.

Release Notes:

- Fixed dev container start silently connecting to a stale container
  when multiple containers matched the project's identifying labels;
  Zed now surfaces a clear error naming the duplicate IDs.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…RED)

Adds a failing assertion to the existing `test_spawns_devcontainer_with_docker_compose` test,
pinning the value Zed should pass to `docker compose --project-name`: the reference
`@devcontainers/cli` derives it as `${workspaceFolderBasename}_devcontainer`, not from
the devcontainer.json `name` field.

The test fixture's devcontainer has `"name": "Rust and PostgreSQL"`, so the current
derivation (safe_id_lower of `name`) produces `rust_and_postgresql`, while the
CLI-parity derivation for project basename `project` would produce `project_devcontainer`.

This commit intentionally leaves production code unchanged to lock in the RED state.
The implementation change follows in the next commit.

Failure output:

---- devcontainer_manifest::test::test_spawns_devcontainer_with_docker_compose stdout ----
thread '...' panicked at crates/dev_container/src/devcontainer_manifest.rs:3558:9:
assertion `left == right` failed: compose project name should match @devcontainers/cli
derivation (${folderBasename}_devcontainer), ignoring devcontainer.json `name`
  left: "rust_and_postgresql"
 right: "project_devcontainer"

Refs: #5
Change `DevContainerManifest::project_name()` to derive the Docker Compose
project name as `${workspaceFolderBasename}_devcontainer`, matching
`@devcontainers/cli`'s `toProjectName` in src/spec-node/dockerCompose.ts.
Previously Zed used `safe_id_lower(devcontainer.json's name)` when the
`name` field was set, which diverged from the reference CLI and produced
two separate compose projects for the same folder when both tools were
used against it.

Concretely, for a folder `devcontainer-compose-test/` with
`"name": "Compose Duplicate Repro"`:
- Old Zed: `compose_duplicate_repro`
- CLI / VS Code: `devcontainer-compose-test_devcontainer`
- New Zed: `devcontainer-compose-test_devcontainer` (matches CLI)

Adds `sanitize_compose_project_name()` helper which implements the CLI's
exact sanitization (lowercase + strip `[^-_a-z0-9]`) — notably preserving
hyphens, which `safe_id_lower` would have replaced with underscores.

Satisfies the RED assertion from the previous commit and adds a focused
unit test covering hyphens, uppercase, and special-char stripping.

The `name` field is still used as the features image-tag prefix
(`generate_features_image_tag`); only the compose project namespace is
decoupled from it.

Label-based container lookup (`devcontainer.local_folder` +
`devcontainer.config_file`) is unchanged, so an upgrading user's
existing containers are still found and reused; only new creations use
the new derivation. No migration step is required.

Fixes #5

Release Notes:

- Fixed dev container Docker Compose project name now matches the reference devcontainer CLI (`${folderBasename}_devcontainer`), preventing duplicate containers when the same folder is opened with both Zed and the devcontainer CLI / VS Code, or when upgrading from v0.231.x to v0.232+ on an existing Zed-managed workspace.
Scaffolds a test for the multi-match upgrade scenario where one of the
duplicate containers lives under the canonical (reference-CLI-matching)
compose project and the other under a legacy name. The tiebreak logic
that would prefer the canonical container does not exist yet, so
`check_for_existing_container_prefers_canonical_compose_project` fails
with `MultipleMatchingContainers`. The companion safety-net test
(`check_for_existing_container_errors_when_none_canonical`) passes
already and guards against regressions in the zero-canonical case.

This commit is deliberately test-only on the logic layer; the prod-code
changes are purely structural and required to let the tests compile:

- Extend `DockerConfigLabels` with `compose_project: Option<String>`
  serde-renamed to `com.docker.compose.project`. All existing inspect
  struct literals (3 in docker.rs tests, 8 in devcontainer_manifest.rs
  tests) get `compose_project: None`.
- Add `FakeDocker::inspect_overrides: Mutex<HashMap<String, DockerInspect>>`
  and `add_inspect_override` setter. `FakeDocker::inspect` consults
  overrides before its hardcoded pattern matching, so tests can control
  each multi-match candidate's `com.docker.compose.project` label
  without disturbing any existing test fixture.

RED output (`cargo test -p dev_container --lib check_for_existing_container`):

    running 3 tests
    test devcontainer_manifest::test::check_for_existing_container_errors_when_multiple_match ... ok
    test devcontainer_manifest::test::check_for_existing_container_errors_when_none_canonical ... ok
    test devcontainer_manifest::test::check_for_existing_container_prefers_canonical_compose_project ... FAILED

    ---- devcontainer_manifest::test::check_for_existing_container_prefers_canonical_compose_project stdout ----
    thread 'devcontainer_manifest::test::check_for_existing_container_prefers_canonical_compose_project' panicked at crates/dev_container/src/devcontainer_manifest.rs:5043:13:
    expected Ok(Some(canonical)), got Err(MultipleMatchingContainers(["canonical_id", "legacy_id"]))

    test result: FAILED. 2 passed; 1 failed

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Introduce `pick_canonical_container`, a thin recovery layer above
`find_process_by_filters`. When the multi-match detection from zed-industries#54068
trips, inspect each candidate and prefer the one whose
`com.docker.compose.project` label equals `self.project_name()`. Zero
or ≥2 canonical matches still fall through to the
`MultipleMatchingContainers` error, preserving the safety net; only the
unambiguous-recovery case is intercepted.

This makes the compose-project-name fix from PR #6 a transparent
upgrade: users migrating past v0.231.x to v0.232+ on an existing
Zed-managed project had one container under the legacy
`safe_id_lower(name)` project. After the derivation change new Zed
creates one under the canonical `${folder}_devcontainer`. Without the
tiebreak, the label-based lookup sees both and errors out; with it,
Zed reuses the canonical one and logs the orphan's id so users can
clean up on their own schedule.

The multi-match detection itself (`parse_find_process_output`,
`MultipleMatchingContainers`, its Display impl, the pass-through arm
in `start_dev_container_with_config`) stays byte-identical with
zed-industries#54068.

Updates the pre-existing
`check_for_existing_container_errors_when_multiple_match` test to
supply non-canonical inspect overrides, so it still exercises the
safety-net fall-through now that the path inspects each candidate.

    running 3 tests
    test devcontainer_manifest::test::check_for_existing_container_errors_when_multiple_match ... ok
    test devcontainer_manifest::test::check_for_existing_container_errors_when_none_canonical ... ok
    test devcontainer_manifest::test::check_for_existing_container_prefers_canonical_compose_project ... ok

    test result: ok. 78 passed; 0 failed (full crate)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Locks in the `com.docker.compose.project` serde rename on
`DockerConfigLabels`. The multi-match tiebreak in
`check_for_existing_container` reads this field from real
`docker inspect` output, but every tiebreak test plants the value via
`FakeDocker::add_inspect_override` — nothing exercises the
deserialization path. Without this test, breaking the rename fails
closed silently (tiebreak sees `None`, falls through to
`MultipleMatchingContainers`) and no existing test catches it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Only the multi-match case is intercepted; everything else passes through
unchanged. Using `result => result` makes that intent obvious, rather
than spelling out `Ok(v) => Ok(v)` and `Err(other) => Err(other)` which
the reader has to confirm is a no-op.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The current `project_name()` derives `${folderBasename}_devcontainer`
unconditionally. The reference devcontainers/cli
(`src/spec-node/dockerCompose.ts`'s `getProjectName`) actually walks a
four-step precedence:

  1. `COMPOSE_PROJECT_NAME` env var.
  2. `COMPOSE_PROJECT_NAME` from the workspace `.env` file.
  3. The merged compose config's top-level `name:`, except when it
     equals the literal `"devcontainer"` (Compose's own default when no
     fragment declared `name:`) — in which case fall through to rule 4.
  4. Basename of the first compose file's directory, appending
     `_devcontainer` only when that directory is
     `<workspace>/.devcontainer`.

Zed's partial-match derivation still diverges from the CLI whenever a
user sets `COMPOSE_PROJECT_NAME`, has a `.env` with one, uses top-level
`name:`, or points `dockerComposeFile` outside `.devcontainer/` (e.g.
`"../docker-compose.yml"`). Each produces a second compose project for
the same folder — the very bug this PR is meant to fix.

Adds a pure `derive_project_name` helper as a stub covering only the
workspace-basename-with-suffix branch, plus five focused tests pinning
each rule the stub does not yet honor. GREEN will wire in the full
precedence and convert `project_name()` to async so it can read env,
`.env`, and the merged compose config.

The 5th test (`treats_compose_default_name_as_unset`) guards the rule-3
edge case up-front: a future naive rule-3 implementation that just
forwards `compose_config_name` unconditionally would return
`"devcontainer"` here instead of `"myworkspace_devcontainer"` and the
test would fail. The stub's output happens to satisfy this test by
coincidence (it always returns `<fallback>_devcontainer`), so only the
other four tests are proving RED against the stub.

RED output:

  test derive_project_name_env_wins_over_everything ... FAILED
  test derive_project_name_dotenv_wins_over_compose_and_fallback ... FAILED
  test derive_project_name_compose_name_wins_over_fallback ... FAILED
  test derive_project_name_omits_suffix_when_compose_file_outside_devcontainer_dir ... FAILED
  test derive_project_name_treats_compose_default_name_as_unset ... ok

  left: "project_devcontainer"
  right: "from_env" / "from_dotenv" / "mycomposeproject" / "project"

  test result: FAILED. 1 passed; 4 failed; 0 ignored; 0 measured;
  79 filtered out.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Fills in `derive_project_name` to walk `@devcontainers/cli`'s four-step
precedence from `getProjectName` in `src/spec-node/dockerCompose.ts`:

  1. `COMPOSE_PROJECT_NAME` in the local environment.
  2. `COMPOSE_PROJECT_NAME` in the workspace `.env` file.
  3. Top-level `name:` from the merged compose config — but only when at
     least one compose fragment explicitly declared it. `docker compose
     config` injects `name: devcontainer` into the merged output whenever
     no fragment declared one, and the CLI re-scans fragments to distinguish
     that default from a user-provided name.
  4. Basename of the first compose file's directory, appending
     `_devcontainer` only when that directory is
     `<workspace_root>/.devcontainer`.

Each branch pipes through `sanitize_compose_project_name` — the CLI's
final normalization step (lowercase + strip `[^-_a-z0-9]`).

Converts `project_name()` to `async fn` so it can load the `.env` file and
each compose fragment via `self.fs.load`. Four call sites (`docker compose
build` in both up paths, `docker compose config`, and the tiebreak key in
`pick_canonical_container`) now await the derivation.

Adds `parse_dotenv_compose_project_name` (line scan matching the CLI's
regex-level dotenv subset) and `compose_fragment_declares_name` (line
scan for a top-level `name:` key in block-style YAML). The fragment
scanner is intentionally not a full YAML parser: on exotic YAML (flow-root
mappings, anchors, multi-doc streams) it errs toward rule 4 — the CLI's
own fallback when fragment parsing fails.

RED→GREEN delta (`derive_project_name` test group):

  pre-GREEN (with stub):
    derive_project_name_env_wins_over_everything                         FAILED
    derive_project_name_dotenv_wins_over_compose_and_fallback            FAILED
    derive_project_name_compose_name_wins_over_fallback                  FAILED
    derive_project_name_omits_suffix_when_compose_file_outside_...       FAILED
    derive_project_name_treats_compose_default_name_as_unset             ok
    (1 passed by coincidence against the `<fallback>_devcontainer` stub;
     renamed in GREEN to `_skips_compose_name_when_not_explicitly_declared`
     to reflect the new `compose_name_explicitly_declared: bool` signal.)

  post-GREEN:
    derive_project_name_env_wins_over_everything                         ok
    derive_project_name_dotenv_wins_over_compose_and_fallback            ok
    derive_project_name_compose_name_wins_over_fallback                  ok
    derive_project_name_omits_suffix_when_compose_file_outside_...       ok
    derive_project_name_skips_compose_name_when_not_explicitly_declared  ok
    compose_fragment_declares_name_detects_top_level_name_key            ok

    test result: ok. 85 passed; 0 failed; 0 ignored; 0 measured; 0 filtered
    out; finished in 0.14s

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Addresses two review findings on the CLI-precedence derivation:

1. `.env` read failures are no longer silently ignored. `getProjectName`
   in `@devcontainers/cli` only treats `ENOENT`/`EISDIR` as "no `.env`"
   and rethrows everything else — a policy we were violating by using
   `self.fs.load(&dotenv_path).await.ok()`. A workspace with an
   unreadable `.env` that contains `COMPOSE_PROJECT_NAME` would have
   fallen back to rule 4 under the old code, re-creating the "second
   compose project for the same repo" bug this PR is trying to fix.
   `project_name()` is now `Result<String, DevContainerError>`; real
   I/O errors from both the `.env` read and the compose-fragment loop
   propagate as `FilesystemError`. Missingness is classified by a
   `is_missing_file_error` helper (unit-tested) that accepts only the
   `NotFound`/`NotADirectory` kinds.

2. `compose_fragment_declares_name` now parses the fragment as YAML and
   checks for a `name` key on the root mapping, matching the CLI's
   `yaml.load(...)` approach. The old line scanner only recognized
   block-style `name:` at column 0 and missed valid Compose styles:

     * quoted keys (`"name": my-project`)
     * flow-style root mappings (`{name: …, services: …}`)

   Both are now covered by tests that the line scanner would have
   failed. On YAML parse failure we still fall through to "not
   declared", matching the CLI's own fallback.

Four `project_name()` call sites (`docker_compose_build` in both up
paths, the `docker compose config` command assembly, and the tiebreak
key in `pick_canonical_container`) propagate with `?`. All already live
in `Result<_, DevContainerError>` contexts, so no call-site signature
changes were needed. Adds `anyhow` and `serde_yaml` as direct
`dev_container` dependencies (transitively present already).

  test result: ok. 86 passed; 0 failed; 0 ignored; 0 measured; 0
  filtered out; finished in 0.13s

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The fragment rescan in `project_name()` was propagating every non-missing
read error as `FilesystemError`, which diverges from the reference CLI.
`getProjectName` in `@devcontainers/cli`'s `dockerCompose.ts` (lines
663-673) wraps both the fragment `readFile` and the `yaml.load` in one
try/catch and ignores every failure — the comment there cites custom
tags like `!reset` as one trigger, but the policy is "on any failure,
treat the fragment as not-declared and keep scanning." Under the prior
code, if a fragment became unreadable between `docker compose config`
and the explicit-`name:` check, Zed would fail the whole devcontainer
flow for something the CLI would have silently skipped.

Switches the loop to log + continue on any `fs.load` failure. The `.env`
read in the same function keeps its strict NotFound/NotADirectory-only
policy — that mirrors a separate CLI branch with its own narrow
`ENOENT`/`EISDIR` catch. The `is_missing_file_error` doc comment is
updated to reflect that it now covers only the `.env` path, not the
fragment loop.

  test result: ok. 86 passed; 0 failed; 0 ignored; 0 measured; 0
  filtered out; finished in 0.24s

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Review feedback: the prior shape split the "how many canonical matches?"
decision across a mid-loop early-exit (`if canonical.is_some()`) and a
post-loop match, with the same `canonical.is_some()` check reading
confusingly in both places. Partition all ids into canonical vs. other
first, then decide once based on `canonical_ids.as_slice()`: exactly
one → reuse and log the legacy orphans; zero or two+ → surface
`MultipleMatchingContainers`. Costs one extra `inspect` call in the
two-canonical case (realistic ids.len() is <=3), buys a single,
linear decision point.

Also retitle the doc comment from "common upgrade path" to "narrow
case" — the scenario (new-Zed-first creates a name-field container,
then a CLI-derivation tool runs against the same folder) is specific,
not a general upgrade reproducer.

86 tests pass; cargo fmt and ./script/clippy -p dev_container clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
dtolnay's serde_yaml is archived and publishes as
0.9.34+deprecated. Switch to serde_yaml_ng, an active minimal fork
(acatton) with attribution intact, declared drop-in compatibility with
serde_yaml's Value/from_str/Mapping API, and no outstanding community
concerns. Call sites in compose_fragment_declares_name are renamed
1:1; no behavior change.

Rejected alternatives: serde_yml (sebastienrousseau) — archived, sole
maintainer, community concerns about stripped attribution from the
dtolnay original. Hand-rolling a YAML scanner — the whole reason we
introduced a parser in commit ebe5202 was that line scans miss
quoted keys and flow-style root mappings.

86 tests pass; cargo fmt and ./script/clippy -p dev_container clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Review feedback: derive_project_name has two gaps vs. `getProjectName`
in `src/spec-node/dockerCompose.ts`.

1. `docker_compose_manifest()` builds compose file paths by joining
   `self.config_directory` with each raw `dockerComposeFile` entry
   verbatim, so entries like "subdir/../docker-compose.yml" yield a
   `PathBuf` with unresolved `..`. `derive_project_name` compares
   `parent()` against `<workspace>/.devcontainer` and extracts
   `file_name()` from that raw parent — no normalization — so a file
   semantically under `.devcontainer` takes the wrong branch. Rust's
   `Path::file_name` returns `None` on a path ending in `..`, which
   collapses to `workspace_fallback` bare (not even the CLI's
   rule-4 `${workspace}_devcontainer`).

2. `is_missing_file_error` comment and doc mirror the CLI's narrow
   `ENOENT`/`EISDIR` swallow for the `.env` read, but the code catches
   `NotADirectory` (ENOTDIR) instead of `IsADirectory` (EISDIR). The
   prior unit test locked in the mismatch.

Failing output captured before fix:

    running 2 tests
    test …::derive_project_name_normalizes_compose_path_for_rule_4 ... FAILED
    test …::is_missing_file_error_only_accepts_notfound_and_isadirectory ... FAILED

    ---- derive_project_name_normalizes_compose_path_for_rule_4 ----
    assertion `left == right` failed
      left: "project"
     right: "project_devcontainer"

    ---- is_missing_file_error_only_accepts_notfound_and_isadirectory ----
    assertion failed: is_missing_file_error(&is_a_dir)

    test result: FAILED. 0 passed; 2 failed; 0 ignored.

GREEN lands in the next commit.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Fixes the two bugs pinned by 6d41917:

1. `derive_project_name` now normalizes the compose-file parent and the
   `<workspace>/.devcontainer` target path via `util::normalize_path`
   before comparing. `docker_compose_manifest()` joins
   `self.config_directory` with each raw `dockerComposeFile` entry
   verbatim, so paths can carry `..` components. Without normalization,
   `subdir/../docker-compose.yml` under `.devcontainer/` missed rule 4
   and `.devcontainer/../docker-compose.yml` had `None` from
   `file_name()` on the parent. The reference CLI's `getProjectName`
   does `path.resolve()` before its own branch, so this matches
   upstream.

2. `is_missing_file_error` now matches `IsADirectory` (EISDIR) instead
   of `NotADirectory` (ENOTDIR). The reference CLI's narrow `.env` swallow
   is `ENOENT || EISDIR`; the previous code swallowed `ENOENT || ENOTDIR`,
   which meant a `.env` that was actually a directory would propagate as
   a hard `FilesystemError` instead of falling through to rule 3–5.

Both RED tests from 6d41917 now pass. Full crate green: 87 passed.
Revert the pick_canonical_container stack. When multiple containers match
the identifying labels, propagate MultipleMatchingContainers (introduced in
the detection commit) untouched rather than trying to prefer one. The
mixed-version case — legacy and new containers coexisting — is a transient
edge that does not justify permanent code. Users with duplicate legacy
containers can clean them up once; the detection error surfaces the
mismatch with instructions.

Removes:
- pick_canonical_container method
- DockerConfigLabels.compose_project field and its serde rename
- Tests and helpers specific to the tiebreak

Per review on zed-industries#54302.
Replace serde_yaml_ng with yaml-rust2 for the single call site in
compose_fragment_declares_name. yaml-rust2 is already a transitive
workspace dependency (via tree-sitter-yaml and, until this commit,
serde_yaml_ng → unsafe-libyaml), so this is a net reduction of one
direct workspace dependency. The existing test covering block-style,
quoted-key, flow-style, and parse-failure inputs passes unchanged.

Per review on zed-industries#54302.
@antont
antont force-pushed the fix-compose-project-name-derivation branch from 2418589 to ca9e505 Compare April 22, 2026 15:03
@antont

antont commented Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

@KyleBarton

YAML parser. Accepted. Swapped to yaml-rust2. It was already transitive in the workspace (via tree-sitter-yaml and, previously, serde_yaml_ng → unsafe-libyaml), so this is a net reduction of one direct workspace dep — and it'll line up with #53922 when that lands.

tiebreak + #54068. Accepted. Dropped the pick_canonical_container stack and its com.docker.compose.project serde label. The mixed-legacy-container edge is transient enough to handle via the explicit MultipleMatchingContainers error (from #54068) rather than permanent tiebreaking code. Folded #54068 in and closed it unmerged — the detection commit is on this branch now.

Also rebased onto main, targeted that and updated the PR description accordingly.

Remove test_deserialize_single_json_object — it just tests that serde
deserializes a JSON object, which is serde's responsibility, not ours.
The remaining three tests still lock in the actual contract:
NDJSON is rejected, empty output is None, empty object is None.
@antont antont changed the title dev_container: Align compose project name with reference CLI and recover from mixed-version duplicates dev_container: Align compose project name with reference CLI Apr 22, 2026
@antont

antont commented Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

Re-verified end-to-end against the revised branch with antont/zed-devcontainer-compose-test:

  • Zed opens the fixture and creates container under project devcontainer-compose-test_devcontainer (CLI rule 4).
  • devcontainer up --workspace-folder $PWD from a separate shell reports "containerId": "369feedbbcec..." — the exact container Zed is using — and "composeProjectName": "devcontainer-compose-test_devcontainer". No second compose project spawned.

@KyleBarton KyleBarton left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good - I had one comment about normalizing filepaths earlier in the call path. Really appreciate you digging in here!

// from `file_name()`.
let compose_dir = first_compose_file
.and_then(Path::parent)
.map(normalize_path);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you feel about just doing this for all files upfront in docker_compose_manifest instead? I think it would be better and lead to fewer surprises in the future

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KyleBarton seems like a good idea, thanks, am giving it a shot!

@antont antont Apr 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 1baf48d. Moved the normalization into docker_compose_manifest so every downstream consumer of DockerComposeResources.files (fragment reads, project-name derivation, docker compose -f invocations) sees resolved paths. derive_project_name now just reads first_compose_file.and_then(Path::parent); dropped the redundant comment too.

Existing derive_project_name_handles_resolved_paths_from_docker_compose_manifest test reframed to pin rule-4/rule-5 behavior on the already-normalized paths the helper now receives.

@KyleBarton

Copy link
Copy Markdown
Collaborator

Oh and mind the checkstyle error on spelling - parsable vs parseable

antont and others added 4 commits April 22, 2026 21:18
Move `..`-resolution from `derive_project_name` to `docker_compose_manifest`
so every downstream consumer of `DockerComposeResources.files` (fragment
reads, project-name derivation, `docker compose -f` invocations) sees
resolved paths. Per Kyle's review on zed-industries#54302 (discussion_r3125767714).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Spelling flagged by upstream CI's typos check.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@KyleBarton
KyleBarton self-requested a review April 22, 2026 23:34
@KyleBarton
KyleBarton merged commit f939ec6 into zed-industries:main Apr 22, 2026
31 checks passed
kathbigra pushed a commit to kathbigra/zed that referenced this pull request May 10, 2026
…ustries#54302)

> **Draft / open question for maintainers.** The failure mode this fixes
is narrow — a new-Zed-created container exists under the `name`-field
project while a CLI-derivation tool (`@devcontainers/cli`, VS Code)
operates on the same folder (the container persists in Docker, so the
originating Zed session doesn't need to still be open). See issue zed-industries#54255
failure mode 3 and the fixture's step 6.
>
> I'd like to pose this as a question rather than a claim: is matching
`@devcontainers/cli`'s `getProjectName` precedence something the project
wants to take on, given the narrowness of the bug? I wrote this
implementation mostly as a way to explore what parity would actually
cost — happy to close it if you'd rather leave it as-is, or pare it down
(e.g. just rule 4) if a partial match is preferable.
>
> The broader value beyond this specific bug: devcontainer impls
agreeing on the same project name means containers created by Zed, the
devcontainer CLI, and VS Code are interchangeable for the same folder,
which feels worth it to me — but you know the project's priorities
better.
>
> Folds in zed-industries#54068 (detection) — closing that PR unmerged; its
`MultipleMatchingContainers` error lands here.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes zed-industries#54255

## Summary

**Match `@devcontainers/cli`'s full `getProjectName` precedence.**
Replaces `safe_id_lower(devcontainer.json's name)` with the five-step
chain the reference CLI walks (see [`src/spec-node/dockerCompose.ts` in
devcontainers/cli](https://github.com/devcontainers/cli/blob/main/src/spec-node/dockerCompose.ts)):

1. `COMPOSE_PROJECT_NAME` from the local environment.
2. `COMPOSE_PROJECT_NAME=` in the workspace `.env` file.
3. Top-level `name:` on the merged compose config, when at least one
fragment declared it explicitly.
4. `${workspaceFolderBasename}_devcontainer` — only when the first
compose file's directory is `<workspace>/.devcontainer/`.
5. Otherwise, the plain basename of the first compose file's directory
(no suffix).

The old Zed implementation diverged at every one of those inputs: any
user setting `COMPOSE_PROJECT_NAME`, shipping a `.env` with one,
declaring a top-level compose `name:`, or pointing `dockerComposeFile`
outside `.devcontainer/` (e.g. `"../docker-compose.yml"`) got a
different project namespace than the CLI and VS Code, producing two
compose projects for the same folder.

Adds a small `sanitize_compose_project_name()` helper implementing the
CLI's rules (lowercase + strip `[^-_a-z0-9]`) — notably preserving
hyphens, which `safe_id_lower` would have replaced with underscores.

Adds two helpers used by the precedence walk:
- `parse_dotenv_compose_project_name` — line scan extracting
`COMPOSE_PROJECT_NAME=…` from the workspace `.env`, matching the subset
the CLI's regex dotenv reader recognizes.
- `compose_fragment_declares_name` — parses each compose fragment with
`yaml-rust2` (already a transitive workspace dep; slated to become a
direct dep via zed-industries#53922) and checks for a `name` key on the root mapping
(block, quoted, or flow style all work), matching the CLI's own
`yaml.load`. `docker compose config` always injects `name: devcontainer`
into its merged output when no fragment declared one, so rule 3 needs to
distinguish the user-provided case from the injected default — this
helper supplies that signal. On YAML parse failure it returns "not
declared" (rule 4 applies), matching the CLI's fallback.

`project_name()` becomes async and fallible (`async fn
project_name(&self) -> Result<String, DevContainerError>`) so it can
load the `.env` file and each compose fragment via `self.fs.load`. Four
call sites now `.await?` the derivation. Real I/O errors on the `.env`
read propagate as `FilesystemError` (matching the CLI's narrow
`ENOENT`/`EISDIR` swallow); fragment-rescan read errors are logged and
skipped (matching the CLI's broader try/catch over its fragment read +
parse).

The `name` field is still used as the features image-tag prefix
(`generate_features_image_tag`); only the compose project namespace is
decoupled from it.

**Duplicate-container detection (from zed-industries#54068).** When
`check_for_existing_container`'s label-based lookup returns more than
one match, propagate `MultipleMatchingContainers(ids)` with instructions
to clean up the stale one(s). This covers the mixed-version upgrade edge
case where a pre-fix Zed left a container under the legacy project name
alongside a CLI-style one — transparent to users in the common case (one
tool, one container), explicit error when two legacy siblings need
manual cleanup.

## Why

Full write-up with verified fixtures and captured output: zed-industries#54255.

Three failure modes from the same root cause, all resolved by this
change:

1. **Interop** — opening a folder in both Zed and `devcontainer up` (or
Zed and VS Code) creates two compose projects with identical
`devcontainer.local_folder` + `devcontainer.config_file` labels,
breaking the spec's uniqueness invariant.
2. **Cross-worktree silent db/volume reuse** — if multiple git worktrees
share a `devcontainer.json` with the same `name`, Zed uses the same
compose project for all of them; Compose reuses stateful siblings (db,
cache, localstack) by config-hash, so worktree B silently inherits
worktree A's database. Fixture + captured output:
[antont/zed-devcontainer-db-share-repro](https://github.com/antont/zed-devcontainer-db-share-repro).
3. **Mixed-version Zed sessions** — the Rust impl landed in stable
v0.232.2 (2026-04-15, zed-industries#52338). Older Zed (≤v0.231.x) shelled out to
`@devcontainers/cli` so it used the reference derivation. The collision
shows up when a new-Zed-created container exists under the name-field
project while a CLI-derivation tool (old Zed, `devcontainer up`, VS
Code) operates on the same folder.

## Migration / compatibility

Existing Zed-created containers (under the old `safe_id_lower(name)`
project) continue to be found via `check_for_existing_container`'s
label-based lookup — they're looked up by `devcontainer.local_folder` +
`devcontainer.config_file`, not by project name. A user with duplicate
legacy containers from a prior Zed session sees
`MultipleMatchingContainers` with cleanup instructions.

## Revision — 2026-04-22

Revised per @KyleBarton review on the prior version:
- Swapped the YAML parser from `serde_yaml_ng` to `yaml-rust2` (already
transitive via `tree-sitter-yaml`; net reduction of one direct workspace
dep; also what zed-industries#53922 will pull in).
- Dropped the mixed-version tiebreak (`pick_canonical_container`) and
its `com.docker.compose.project` serde label. The edge case it covered
is transient enough to address via the explicit
`MultipleMatchingContainers` error rather than permanent tiebreaking
code.
- Folded zed-industries#54068's detection commit into this PR; zed-industries#54068 closed unmerged.
- Rebased onto `main`.

## Test plan

- [x] `cargo test -p dev_container --lib` — 89 passed, including:
  - `sanitize_compose_project_name_matches_cli_rules`
- `--project-name` assertion added to
`test_spawns_devcontainer_with_docker_compose`
  - `check_for_existing_container_errors_when_multiple_match`
  - `derive_project_name_env_wins_over_everything`
  - `derive_project_name_dotenv_wins_over_compose_and_fallback`
  - `derive_project_name_compose_name_wins_over_fallback`
- `derive_project_name_skips_compose_name_when_not_explicitly_declared`
-
`derive_project_name_omits_suffix_when_compose_file_outside_devcontainer_dir`
  - `derive_project_name_normalizes_compose_path_for_rule_4`
- `compose_fragment_declares_name_detects_top_level_name_key` (covers
block, quoted-key, and flow-style roots, plus parse failure →
not-declared)
  - `is_missing_file_error_only_accepts_notfound_and_isadirectory`
- [x] `cargo fmt --all` — clean
- [x] `./script/clippy -p dev_container` — clean
- [x] **End-to-end with fixture**
[antont/zed-devcontainer-compose-test](https://github.com/antont/zed-devcontainer-compose-test):
  - Build `zed` from this branch.
- Clean slate: `docker ps -a --filter
"label=devcontainer.local_folder=$PWD" -q | xargs -r docker rm -f`
- `zed --dev-container /path/to/devcontainer-compose-test` → Zed creates
container under project `devcontainer-compose-test_devcontainer` (was
`compose_duplicate_repro` before the fix).
- `devcontainer up --workspace-folder $PWD` → CLI reports the same
`containerId` Zed created; no second compose project is introduced.
- Captured: `devcontainer-compose-test_devcontainer-app-1`,
`composeProjectName: "devcontainer-compose-test_devcontainer"` reported
by both tools.

Release Notes:

- Fixed dev container Docker Compose project name now matches the full
`getProjectName` precedence from the reference devcontainer CLI
(`COMPOSE_PROJECT_NAME` in the environment, then in the workspace
`.env`, then an explicit top-level `name:` on the merged compose config,
then the basename of the first compose file's directory — with the
`_devcontainer` suffix only when that directory is
`<workspace>/.devcontainer`). This prevents duplicate containers when
the same folder is opened with both Zed and the devcontainer CLI / VS
Code.

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
jonx pushed a commit to jonx/zed-aros that referenced this pull request Jul 17, 2026
…ustries#54302)

> **Draft / open question for maintainers.** The failure mode this fixes
is narrow — a new-Zed-created container exists under the `name`-field
project while a CLI-derivation tool (`@devcontainers/cli`, VS Code)
operates on the same folder (the container persists in Docker, so the
originating Zed session doesn't need to still be open). See issue zed-industries#54255
failure mode 3 and the fixture's step 6.
>
> I'd like to pose this as a question rather than a claim: is matching
`@devcontainers/cli`'s `getProjectName` precedence something the project
wants to take on, given the narrowness of the bug? I wrote this
implementation mostly as a way to explore what parity would actually
cost — happy to close it if you'd rather leave it as-is, or pare it down
(e.g. just rule 4) if a partial match is preferable.
>
> The broader value beyond this specific bug: devcontainer impls
agreeing on the same project name means containers created by Zed, the
devcontainer CLI, and VS Code are interchangeable for the same folder,
which feels worth it to me — but you know the project's priorities
better.
>
> Folds in zed-industries#54068 (detection) — closing that PR unmerged; its
`MultipleMatchingContainers` error lands here.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes zed-industries#54255

## Summary

**Match `@devcontainers/cli`'s full `getProjectName` precedence.**
Replaces `safe_id_lower(devcontainer.json's name)` with the five-step
chain the reference CLI walks (see [`src/spec-node/dockerCompose.ts` in
devcontainers/cli](https://github.com/devcontainers/cli/blob/main/src/spec-node/dockerCompose.ts)):

1. `COMPOSE_PROJECT_NAME` from the local environment.
2. `COMPOSE_PROJECT_NAME=` in the workspace `.env` file.
3. Top-level `name:` on the merged compose config, when at least one
fragment declared it explicitly.
4. `${workspaceFolderBasename}_devcontainer` — only when the first
compose file's directory is `<workspace>/.devcontainer/`.
5. Otherwise, the plain basename of the first compose file's directory
(no suffix).

The old Zed implementation diverged at every one of those inputs: any
user setting `COMPOSE_PROJECT_NAME`, shipping a `.env` with one,
declaring a top-level compose `name:`, or pointing `dockerComposeFile`
outside `.devcontainer/` (e.g. `"../docker-compose.yml"`) got a
different project namespace than the CLI and VS Code, producing two
compose projects for the same folder.

Adds a small `sanitize_compose_project_name()` helper implementing the
CLI's rules (lowercase + strip `[^-_a-z0-9]`) — notably preserving
hyphens, which `safe_id_lower` would have replaced with underscores.

Adds two helpers used by the precedence walk:
- `parse_dotenv_compose_project_name` — line scan extracting
`COMPOSE_PROJECT_NAME=…` from the workspace `.env`, matching the subset
the CLI's regex dotenv reader recognizes.
- `compose_fragment_declares_name` — parses each compose fragment with
`yaml-rust2` (already a transitive workspace dep; slated to become a
direct dep via zed-industries#53922) and checks for a `name` key on the root mapping
(block, quoted, or flow style all work), matching the CLI's own
`yaml.load`. `docker compose config` always injects `name: devcontainer`
into its merged output when no fragment declared one, so rule 3 needs to
distinguish the user-provided case from the injected default — this
helper supplies that signal. On YAML parse failure it returns "not
declared" (rule 4 applies), matching the CLI's fallback.

`project_name()` becomes async and fallible (`async fn
project_name(&self) -> Result<String, DevContainerError>`) so it can
load the `.env` file and each compose fragment via `self.fs.load`. Four
call sites now `.await?` the derivation. Real I/O errors on the `.env`
read propagate as `FilesystemError` (matching the CLI's narrow
`ENOENT`/`EISDIR` swallow); fragment-rescan read errors are logged and
skipped (matching the CLI's broader try/catch over its fragment read +
parse).

The `name` field is still used as the features image-tag prefix
(`generate_features_image_tag`); only the compose project namespace is
decoupled from it.

**Duplicate-container detection (from zed-industries#54068).** When
`check_for_existing_container`'s label-based lookup returns more than
one match, propagate `MultipleMatchingContainers(ids)` with instructions
to clean up the stale one(s). This covers the mixed-version upgrade edge
case where a pre-fix Zed left a container under the legacy project name
alongside a CLI-style one — transparent to users in the common case (one
tool, one container), explicit error when two legacy siblings need
manual cleanup.

## Why

Full write-up with verified fixtures and captured output: zed-industries#54255.

Three failure modes from the same root cause, all resolved by this
change:

1. **Interop** — opening a folder in both Zed and `devcontainer up` (or
Zed and VS Code) creates two compose projects with identical
`devcontainer.local_folder` + `devcontainer.config_file` labels,
breaking the spec's uniqueness invariant.
2. **Cross-worktree silent db/volume reuse** — if multiple git worktrees
share a `devcontainer.json` with the same `name`, Zed uses the same
compose project for all of them; Compose reuses stateful siblings (db,
cache, localstack) by config-hash, so worktree B silently inherits
worktree A's database. Fixture + captured output:
[antont/zed-devcontainer-db-share-repro](https://github.com/antont/zed-devcontainer-db-share-repro).
3. **Mixed-version Zed sessions** — the Rust impl landed in stable
v0.232.2 (2026-04-15, zed-industries#52338). Older Zed (≤v0.231.x) shelled out to
`@devcontainers/cli` so it used the reference derivation. The collision
shows up when a new-Zed-created container exists under the name-field
project while a CLI-derivation tool (old Zed, `devcontainer up`, VS
Code) operates on the same folder.

## Migration / compatibility

Existing Zed-created containers (under the old `safe_id_lower(name)`
project) continue to be found via `check_for_existing_container`'s
label-based lookup — they're looked up by `devcontainer.local_folder` +
`devcontainer.config_file`, not by project name. A user with duplicate
legacy containers from a prior Zed session sees
`MultipleMatchingContainers` with cleanup instructions.

## Revision — 2026-04-22

Revised per @KyleBarton review on the prior version:
- Swapped the YAML parser from `serde_yaml_ng` to `yaml-rust2` (already
transitive via `tree-sitter-yaml`; net reduction of one direct workspace
dep; also what zed-industries#53922 will pull in).
- Dropped the mixed-version tiebreak (`pick_canonical_container`) and
its `com.docker.compose.project` serde label. The edge case it covered
is transient enough to address via the explicit
`MultipleMatchingContainers` error rather than permanent tiebreaking
code.
- Folded zed-industries#54068's detection commit into this PR; zed-industries#54068 closed unmerged.
- Rebased onto `main`.

## Test plan

- [x] `cargo test -p dev_container --lib` — 89 passed, including:
  - `sanitize_compose_project_name_matches_cli_rules`
- `--project-name` assertion added to
`test_spawns_devcontainer_with_docker_compose`
  - `check_for_existing_container_errors_when_multiple_match`
  - `derive_project_name_env_wins_over_everything`
  - `derive_project_name_dotenv_wins_over_compose_and_fallback`
  - `derive_project_name_compose_name_wins_over_fallback`
- `derive_project_name_skips_compose_name_when_not_explicitly_declared`
-
`derive_project_name_omits_suffix_when_compose_file_outside_devcontainer_dir`
  - `derive_project_name_normalizes_compose_path_for_rule_4`
- `compose_fragment_declares_name_detects_top_level_name_key` (covers
block, quoted-key, and flow-style roots, plus parse failure →
not-declared)
  - `is_missing_file_error_only_accepts_notfound_and_isadirectory`
- [x] `cargo fmt --all` — clean
- [x] `./script/clippy -p dev_container` — clean
- [x] **End-to-end with fixture**
[antont/zed-devcontainer-compose-test](https://github.com/antont/zed-devcontainer-compose-test):
  - Build `zed` from this branch.
- Clean slate: `docker ps -a --filter
"label=devcontainer.local_folder=$PWD" -q | xargs -r docker rm -f`
- `zed --dev-container /path/to/devcontainer-compose-test` → Zed creates
container under project `devcontainer-compose-test_devcontainer` (was
`compose_duplicate_repro` before the fix).
- `devcontainer up --workspace-folder $PWD` → CLI reports the same
`containerId` Zed created; no second compose project is introduced.
- Captured: `devcontainer-compose-test_devcontainer-app-1`,
`composeProjectName: "devcontainer-compose-test_devcontainer"` reported
by both tools.

Release Notes:

- Fixed dev container Docker Compose project name now matches the full
`getProjectName` precedence from the reference devcontainer CLI
(`COMPOSE_PROJECT_NAME` in the environment, then in the workspace
`.env`, then an explicit top-level `name:` on the merged compose config,
then the basename of the first compose file's directory — with the
`_devcontainer` suffix only when that directory is
`<workspace>/.devcontainer`). This prevents duplicate containers when
the same folder is opened with both Zed and the devcontainer CLI / VS
Code.

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
jolutz pushed a commit to jolutz/zed that referenced this pull request Aug 8, 2026
…ustries#54302)

> **Draft / open question for maintainers.** The failure mode this fixes
is narrow — a new-Zed-created container exists under the `name`-field
project while a CLI-derivation tool (`@devcontainers/cli`, VS Code)
operates on the same folder (the container persists in Docker, so the
originating Zed session doesn't need to still be open). See issue zed-industries#54255
failure mode 3 and the fixture's step 6.
>
> I'd like to pose this as a question rather than a claim: is matching
`@devcontainers/cli`'s `getProjectName` precedence something the project
wants to take on, given the narrowness of the bug? I wrote this
implementation mostly as a way to explore what parity would actually
cost — happy to close it if you'd rather leave it as-is, or pare it down
(e.g. just rule 4) if a partial match is preferable.
>
> The broader value beyond this specific bug: devcontainer impls
agreeing on the same project name means containers created by Zed, the
devcontainer CLI, and VS Code are interchangeable for the same folder,
which feels worth it to me — but you know the project's priorities
better.
>
> Folds in zed-industries#54068 (detection) — closing that PR unmerged; its
`MultipleMatchingContainers` error lands here.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes zed-industries#54255

## Summary

**Match `@devcontainers/cli`'s full `getProjectName` precedence.**
Replaces `safe_id_lower(devcontainer.json's name)` with the five-step
chain the reference CLI walks (see [`src/spec-node/dockerCompose.ts` in
devcontainers/cli](https://github.com/devcontainers/cli/blob/main/src/spec-node/dockerCompose.ts)):

1. `COMPOSE_PROJECT_NAME` from the local environment.
2. `COMPOSE_PROJECT_NAME=` in the workspace `.env` file.
3. Top-level `name:` on the merged compose config, when at least one
fragment declared it explicitly.
4. `${workspaceFolderBasename}_devcontainer` — only when the first
compose file's directory is `<workspace>/.devcontainer/`.
5. Otherwise, the plain basename of the first compose file's directory
(no suffix).

The old Zed implementation diverged at every one of those inputs: any
user setting `COMPOSE_PROJECT_NAME`, shipping a `.env` with one,
declaring a top-level compose `name:`, or pointing `dockerComposeFile`
outside `.devcontainer/` (e.g. `"../docker-compose.yml"`) got a
different project namespace than the CLI and VS Code, producing two
compose projects for the same folder.

Adds a small `sanitize_compose_project_name()` helper implementing the
CLI's rules (lowercase + strip `[^-_a-z0-9]`) — notably preserving
hyphens, which `safe_id_lower` would have replaced with underscores.

Adds two helpers used by the precedence walk:
- `parse_dotenv_compose_project_name` — line scan extracting
`COMPOSE_PROJECT_NAME=…` from the workspace `.env`, matching the subset
the CLI's regex dotenv reader recognizes.
- `compose_fragment_declares_name` — parses each compose fragment with
`yaml-rust2` (already a transitive workspace dep; slated to become a
direct dep via zed-industries#53922) and checks for a `name` key on the root mapping
(block, quoted, or flow style all work), matching the CLI's own
`yaml.load`. `docker compose config` always injects `name: devcontainer`
into its merged output when no fragment declared one, so rule 3 needs to
distinguish the user-provided case from the injected default — this
helper supplies that signal. On YAML parse failure it returns "not
declared" (rule 4 applies), matching the CLI's fallback.

`project_name()` becomes async and fallible (`async fn
project_name(&self) -> Result<String, DevContainerError>`) so it can
load the `.env` file and each compose fragment via `self.fs.load`. Four
call sites now `.await?` the derivation. Real I/O errors on the `.env`
read propagate as `FilesystemError` (matching the CLI's narrow
`ENOENT`/`EISDIR` swallow); fragment-rescan read errors are logged and
skipped (matching the CLI's broader try/catch over its fragment read +
parse).

The `name` field is still used as the features image-tag prefix
(`generate_features_image_tag`); only the compose project namespace is
decoupled from it.

**Duplicate-container detection (from zed-industries#54068).** When
`check_for_existing_container`'s label-based lookup returns more than
one match, propagate `MultipleMatchingContainers(ids)` with instructions
to clean up the stale one(s). This covers the mixed-version upgrade edge
case where a pre-fix Zed left a container under the legacy project name
alongside a CLI-style one — transparent to users in the common case (one
tool, one container), explicit error when two legacy siblings need
manual cleanup.

## Why

Full write-up with verified fixtures and captured output: zed-industries#54255.

Three failure modes from the same root cause, all resolved by this
change:

1. **Interop** — opening a folder in both Zed and `devcontainer up` (or
Zed and VS Code) creates two compose projects with identical
`devcontainer.local_folder` + `devcontainer.config_file` labels,
breaking the spec's uniqueness invariant.
2. **Cross-worktree silent db/volume reuse** — if multiple git worktrees
share a `devcontainer.json` with the same `name`, Zed uses the same
compose project for all of them; Compose reuses stateful siblings (db,
cache, localstack) by config-hash, so worktree B silently inherits
worktree A's database. Fixture + captured output:
[antont/zed-devcontainer-db-share-repro](https://github.com/antont/zed-devcontainer-db-share-repro).
3. **Mixed-version Zed sessions** — the Rust impl landed in stable
v0.232.2 (2026-04-15, zed-industries#52338). Older Zed (≤v0.231.x) shelled out to
`@devcontainers/cli` so it used the reference derivation. The collision
shows up when a new-Zed-created container exists under the name-field
project while a CLI-derivation tool (old Zed, `devcontainer up`, VS
Code) operates on the same folder.

## Migration / compatibility

Existing Zed-created containers (under the old `safe_id_lower(name)`
project) continue to be found via `check_for_existing_container`'s
label-based lookup — they're looked up by `devcontainer.local_folder` +
`devcontainer.config_file`, not by project name. A user with duplicate
legacy containers from a prior Zed session sees
`MultipleMatchingContainers` with cleanup instructions.

## Revision — 2026-04-22

Revised per @KyleBarton review on the prior version:
- Swapped the YAML parser from `serde_yaml_ng` to `yaml-rust2` (already
transitive via `tree-sitter-yaml`; net reduction of one direct workspace
dep; also what zed-industries#53922 will pull in).
- Dropped the mixed-version tiebreak (`pick_canonical_container`) and
its `com.docker.compose.project` serde label. The edge case it covered
is transient enough to address via the explicit
`MultipleMatchingContainers` error rather than permanent tiebreaking
code.
- Folded zed-industries#54068's detection commit into this PR; zed-industries#54068 closed unmerged.
- Rebased onto `main`.

## Test plan

- [x] `cargo test -p dev_container --lib` — 89 passed, including:
  - `sanitize_compose_project_name_matches_cli_rules`
- `--project-name` assertion added to
`test_spawns_devcontainer_with_docker_compose`
  - `check_for_existing_container_errors_when_multiple_match`
  - `derive_project_name_env_wins_over_everything`
  - `derive_project_name_dotenv_wins_over_compose_and_fallback`
  - `derive_project_name_compose_name_wins_over_fallback`
- `derive_project_name_skips_compose_name_when_not_explicitly_declared`
-
`derive_project_name_omits_suffix_when_compose_file_outside_devcontainer_dir`
  - `derive_project_name_normalizes_compose_path_for_rule_4`
- `compose_fragment_declares_name_detects_top_level_name_key` (covers
block, quoted-key, and flow-style roots, plus parse failure →
not-declared)
  - `is_missing_file_error_only_accepts_notfound_and_isadirectory`
- [x] `cargo fmt --all` — clean
- [x] `./script/clippy -p dev_container` — clean
- [x] **End-to-end with fixture**
[antont/zed-devcontainer-compose-test](https://github.com/antont/zed-devcontainer-compose-test):
  - Build `zed` from this branch.
- Clean slate: `docker ps -a --filter
"label=devcontainer.local_folder=$PWD" -q | xargs -r docker rm -f`
- `zed --dev-container /path/to/devcontainer-compose-test` → Zed creates
container under project `devcontainer-compose-test_devcontainer` (was
`compose_duplicate_repro` before the fix).
- `devcontainer up --workspace-folder $PWD` → CLI reports the same
`containerId` Zed created; no second compose project is introduced.
- Captured: `devcontainer-compose-test_devcontainer-app-1`,
`composeProjectName: "devcontainer-compose-test_devcontainer"` reported
by both tools.

Release Notes:

- Fixed dev container Docker Compose project name now matches the full
`getProjectName` precedence from the reference devcontainer CLI
(`COMPOSE_PROJECT_NAME` in the environment, then in the workspace
`.env`, then an explicit top-level `name:` on the merged compose config,
then the basename of the first compose file's directory — with the
`_devcontainer` suffix only when that directory is
`<workspace>/.devcontainer`). This prevents duplicate containers when
the same folder is opened with both Zed and the devcontainer CLI / VS
Code.

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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.

Dev container: compose project name derived from name field diverges from devcontainer CLI / VS Code

2 participants