From ea006b68c27f25416220492ab6a318bd35bb8f93 Mon Sep 17 00:00:00 2001 From: Alex Blank Date: Fri, 29 May 2026 11:58:50 +0200 Subject: [PATCH] fix: mount config profile files individually instead of replacing directories The previous sorting fix exposed a deeper bug: ResolvedMount always mounted its staging directory as a single bind mount. When a config profile mount targeted /workspace/x/y and contained a single file z.json, the staging directory (containing only z.json) replaced the ENTIRE /workspace/x/y directory, hiding all sibling files from git repo mounts. - Change apply_resolved_profile to mount each file individually: - source: staging_dir/relative_path - target: expanded_target/relative_path - Sibling files from other mounts are preserved. - Empty mounts produce no volume entries. - Keep volume sorting (parent paths before child paths) which is still necessary for directory mounts and ensures parent dirs exist before file mounts inside them. - Add 4 unit tests for file-level mount behavior. Quality gates: pytest (218 passed, 6 pre-existing), tsc --noEmit (clean) --- .../src/services/config_profile_resolver.py | 17 ++-- .../unit/test_config_profile_resolver.py | 78 +++++++++++++++++++ .../explorations/mount-file-level-overlays.md | 68 ++++++++++++++++ 3 files changed, 156 insertions(+), 7 deletions(-) create mode 100644 openspec/explorations/mount-file-level-overlays.md diff --git a/apps/api/src/services/config_profile_resolver.py b/apps/api/src/services/config_profile_resolver.py index b839214..cdb9e5c 100644 --- a/apps/api/src/services/config_profile_resolver.py +++ b/apps/api/src/services/config_profile_resolver.py @@ -494,13 +494,16 @@ def apply_resolved_profile( full_path.parent.mkdir(parents=True, exist_ok=True) full_path.write_text(content) - volume_mounts.append( - { - "source": str(mount_dir), - "target": expanded_target, - "type": "bind", - } - ) + # Mount each file individually so sibling files from other mounts + # (e.g. git repo directories) are preserved. + file_target = os.path.join(expanded_target, file_path) + volume_mounts.append( + { + "source": str(full_path), + "target": file_target, + "type": "bind", + } + ) return env_vars, files, volume_mounts, resolved.runtime_hints diff --git a/apps/api/tests/unit/test_config_profile_resolver.py b/apps/api/tests/unit/test_config_profile_resolver.py index ead1377..1c6c11e 100644 --- a/apps/api/tests/unit/test_config_profile_resolver.py +++ b/apps/api/tests/unit/test_config_profile_resolver.py @@ -6,6 +6,9 @@ from src.models.config_profile import ConfigProfile, ConfigProfileInclude from src.services.config_profile_resolver import ( ConfigProfileCycleError, ConfigProfileNotFoundError, + ResolvedMount, + ResolvedProfile, + apply_resolved_profile, check_include_cycle, resolve_profile, _merge_env_vars, @@ -479,6 +482,81 @@ class TestResolveProfile: await resolve_profile(db_session, uuid.uuid4()) +class TestApplyResolvedProfile: + """Unit tests for apply_resolved_profile file-level mount behavior.""" + + def test_mounts_individual_files_not_directory(self, tmp_path) -> None: + """Each file in a ResolvedMount should be mounted individually, not the staging dir.""" + resolved = ResolvedProfile( + profile_id=uuid.uuid4(), + profile_name="test", + mounts={ + "/app": ResolvedMount( + target="/app", + mode="rw", + files={"config.json": '{"key": "value"}', "nested/file.txt": "hello"}, + ) + }, + ) + env, files, volumes, hints = apply_resolved_profile(str(tmp_path), resolved) + + assert len(volumes) == 2 + targets = {v["target"] for v in volumes} + assert "/app/config.json" in targets + assert "/app/nested/file.txt" in targets + # No directory-level mount + assert "/app" not in targets + + def test_file_mount_preserves_sibling_files(self, tmp_path) -> None: + """File-level mounts should not hide sibling files from other mounts.""" + resolved = ResolvedProfile( + profile_id=uuid.uuid4(), + profile_name="test", + mounts={ + "/workspace/x/y": ResolvedMount( + target="/workspace/x/y", + mode="rw", + files={"z.json": "override"}, + ) + }, + ) + env, files, volumes, hints = apply_resolved_profile(str(tmp_path), resolved) + + assert len(volumes) == 1 + assert volumes[0]["target"] == "/workspace/x/y/z.json" + assert volumes[0]["source"].endswith("z.json") + + def test_empty_mount_produces_no_volumes(self, tmp_path) -> None: + """A mount with no files should not produce any volume entries.""" + resolved = ResolvedProfile( + profile_id=uuid.uuid4(), + profile_name="test", + mounts={ + "/app": ResolvedMount(target="/app", mode="rw", files={}) + }, + ) + env, files, volumes, hints = apply_resolved_profile(str(tmp_path), resolved) + assert volumes == [] + + def test_home_expansion_in_file_mount_target(self, tmp_path) -> None: + """~ in mount target should be expanded to home_dir for file mounts.""" + resolved = ResolvedProfile( + profile_id=uuid.uuid4(), + profile_name="test", + mounts={ + "~/.config": ResolvedMount( + target="~/.config", + mode="rw", + files={"app.toml": "setting = 1"}, + ) + }, + ) + env, files, volumes, hints = apply_resolved_profile( + str(tmp_path), resolved, home_dir="/home/user" + ) + assert volumes[0]["target"] == "/home/user/.config/app.toml" + + class TestCheckIncludeCycle: """Unit tests for include cycle checking.""" diff --git a/openspec/explorations/mount-file-level-overlays.md b/openspec/explorations/mount-file-level-overlays.md new file mode 100644 index 0000000..411100e --- /dev/null +++ b/openspec/explorations/mount-file-level-overlays.md @@ -0,0 +1,68 @@ +# Exploration: File-Level Mount Overlays + +## Follow-up to Mount Specificity Ordering + +The sorting fix (parent paths before child paths) was correct for directory mounts, +but it exposed a deeper bug: `ResolvedMount` always mounts its staging directory +as a single bind mount. When a config profile mount targets `/workspace/x/y` and +contains a single file `z.json`, the staging directory (containing only `z.json`) +replaces the ENTIRE `/workspace/x/y` directory, hiding all sibling files from the +git repo. + +## Root Cause + +In `apply_resolved_profile`: +```python +volume_mounts.append({ + "source": str(mount_dir), # staging dir with ONLY z.json + "target": expanded_target, # /workspace/x/y + "type": "bind", +}) +``` + +This mounts a directory. Docker bind mounts at a directory path completely replace +the target directory. There is no merge. + +## What the User Expects + +Git repo mount: `/repo/x` → `/workspace/x` (directory with many files) +Config profile mount: `z.json` → `/workspace/x/y/z.json` (single file overlay) + +Expected: `/workspace/x/y/` contains all repo files PLUS the overlaid `z.json`. +Actual (before sorting): parent mount hides child mount (child never visible). +Actual (after sorting): child directory mount replaces parent subdirectory +(`/workspace/x/y` now contains ONLY `z.json`). + +## Solution + +Mount each file individually instead of the staging directory. + +```python +for file_path, content in mount.files.items(): + full_path = mount_dir / file_path + full_path.write_text(content) + volume_mounts.append({ + "source": str(full_path), + "target": os.path.join(expanded_target, file_path), + "type": "bind", + }) +``` + +This creates file-level bind mounts. Docker mounts a single file without affecting +sibling files in the parent directory. + +## Edge Cases + +- Empty `files` dict: skip, no mounts created. +- Nested file paths (`a/b/c.txt`): mount `staging/a/b/c.txt` → `target/a/b/c.txt`. + Docker creates parent directories as needed. +- File target already exists in git repo: file mount wins (desired override behavior). +- No git repo mount (directory doesn't exist in image): Docker creates parent dirs + for the first file mount. + +## Sorting Fix Status + +KEEP the sorting. It is still correct and necessary for cases where directory +mounts genuinely override parent directories (e.g., a git mount to `/workspace/x` +and another git mount to `/workspace/x/sub`). File-level mounts also benefit from +sorting because the parent directory mount must exist before file mounts inside it.