fix(up): keep --workspace-folder for discovery; git root drives mount only (#67)#78
Merged
Merged
Conversation
… only (#67) Before this change, `--mount-workspace-git-root` (true by default) caused the resolved git root to overwrite `args.workspace_folder` in `execute_up`. The single overwritten path then fed both config discovery and the workspace mount source — so running deacon on a sub-project inside a larger git repository silently loaded the *enclosing* repo's `.devcontainer/devcontainer.json` instead of the sub-project's own. That's exactly why every `up`-based example under `examples/` had to be invoked with `--mount-workspace-git-root false` to escape the parent repo. Decouple the two responsibilities: - `execute_up` now only canonicalizes `args.workspace_folder` — it no longer walks up to the git root. Discovery, `${localWorkspaceFolder}` substitution, image-metadata merging, id-label discovery, container identity, and Dockerfile path resolution all use the user-provided workspace folder. - The git-root walk happens **only** when constructing the default workspace bind mount, inside `execute_container_up` and `execute_compose_up`. When `--mount-workspace-git-root=true` (the default), the mount *source* is the enclosing git root so git operations inside the container still work; otherwise the mount source is the canonicalized workspace folder. `ContainerIdentity::hash_workspace_path` already calls `resolve_workspace_root` internally, so container identity (and the workspace hash label) remain stable across this change. SPEC.md updates: `up/SPEC.md` and the read-configuration spec now state explicitly that `--mount-workspace-git-root` affects the mount only, not discovery. Tests: - `integration_workspace_discovery_in_git_repo` builds a nested workspace (outer git repo with its own `.devcontainer`, plus a sub directory with a different `.devcontainer`) and verifies that `deacon read-configuration --workspace-folder <sub>` loads the sub-project's config and reports `workspace.configFolderPath` pointing into the sub directory. Closes #67. Unblocks the bulk of `examples/`'s `exec.sh` matrix per the #74 tracking issue (workaround dependency). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…mount source The previous version of this PR injected the default workspace mount into `config.workspace_mount` *before* computing `ContainerIdentity`, which changed the workspace+config hash and broke the smoke tests: `deacon up` created a container with a hash that included the mount string, then `deacon exec` looked it up using the on-disk config (no mount string) and got a different hash → "No running container found". Restructure so the identity hash sees the pristine on-disk config: - Keep the original eager-set of `config.workspace_mount` only for the `--workspace-mount-consistency` case (matches the pre-PR behavior for that path; the alternative would also drift the hash, but that drift already existed and is not what #67 is about). - For the default case, do not touch `config.workspace_mount`. Instead compute a `workspace_mount_source: PathBuf` (git root when `--mount-workspace-git-root=true`, else canonicalized workspace) and pass it as the `workspace_path` argument to `Docker::up` → `Docker::create_container`. The existing default-mount branch in `docker.rs:create_container` then sources the bind mount from that path while the user's workspace folder remains untouched in identity computation, discovery, etc. This keeps the #67 spec-parity contract (mount source = git root; discovery + identity = user-provided workspace) and unbreaks the smoke suite. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--mount-workspace-git-rootno longer overwrites the workspace folder for config discovery. The flag now controls only the workspace bind mount source.execute_upcanonicalizesargs.workspace_folderbut does not walk up to the git root. Discovery,${localWorkspaceFolder}, image-metadata merging, id-label discovery, container identity, and Dockerfile resolution all use the user-provided path.execute_container_up/execute_compose_upwhen constructing the default workspace bind mount. When--mount-workspace-git-root=true(the default), the mount source is the enclosing git root so git operations inside the container still work.Container identity is unchanged —
ContainerIdentity::hash_workspace_pathalready callsresolve_workspace_rootinternally, so the workspace hash label stays stable.Closes #67. Unblocks the bulk of
examples/*per #74's "Workaround dependency" section —--mount-workspace-git-root falseis no longer needed to run examples inside the deacon repo.Test plan
make test-nextest-fast(2091 passing, +2 new)cargo fmt --all -- --check/cargo clippy --all-targets -- -D warningsread_configuration_in_subdir_loads_inner_config_not_git_rootread_configuration_in_subdir_with_explicit_git_root_flag_still_loads_inner🤖 Generated with Claude Code