[Bugfix] Drop service/2/ from the service configuration during dstack preset export

src/dstack/_internal/cli/services/presets/export.py

@@ -6,6 +6,7 @@
 from dstack._internal.cli.models.presets import VerifiedPreset
 from dstack._internal.cli.services.presets.build import service_configuration_to_yaml_dict
 from dstack._internal.core.errors import CLIError
+from dstack._internal.core.models.files import FilePathMapping
 
 
 def export_preset(
@@ -16,20 +17,26 @@ def export_preset(
     force: bool,
 ) -> list[Path]:
     """Writes the preset's service as a `type: service` configuration at
-    `destination` and copies the files it references next to it, keeping their
-    relative paths, so the result deploys with plain `dstack apply -f`.
-    Returns every path written."""
+    `destination` and copies the files it references next to it, so the result
+    deploys with plain `dstack apply -f`. Returns every path written."""
     service = preset.service.model_copy(deep=True)
-    copies: list[tuple[Path, Path]] = []
+    stored: list[FilePathMapping] = []
+    record_paths: list[Path] = []
     for mapping in service.files:
         source = Path(mapping.local_path)
         # Loading resolved these against the preset directory; a file stored
         # outside it keeps its absolute path and needs no copy.
         if not source.is_relative_to(preset_dir):
             continue
-        relative = source.relative_to(preset_dir)
-        copies.append((source, destination.parent / relative))
-        mapping.local_path = relative.as_posix()
+        stored.append(mapping)
+        record_paths.append(source.relative_to(preset_dir))
+    exported_paths = [_without_record_prefix(path) for path in record_paths]
+    if _collides(exported_paths, record_paths, destination):
+        exported_paths = record_paths
+    copies: list[tuple[Path, Path]] = []
+    for mapping, record_path, exported_path in zip(stored, record_paths, exported_paths):
+        copies.append((preset_dir / record_path, destination.parent / exported_path))
+        mapping.local_path = exported_path.as_posix()
     written = [destination] + [target for _, target in copies]
     if not force:
         for target in written:
@@ -46,3 +53,35 @@ def export_preset(
         target.parent.mkdir(parents=True, exist_ok=True)
         shutil.copy2(source, target)
     return written
+
+
+def _without_record_prefix(relative: Path) -> Path:
+    """The store keeps a service's files inside the session records it mirrors,
+    `service/<k>/` (final-service attempt) or `trials/<n>/`; the numbering is
+    internal, so the export keeps only the structure under it:
+    `service/2/patches/fix.patch` exports as `patches/fix.patch`."""
+    if (
+        len(relative.parts) > 2
+        and relative.parts[0] in ("service", "trials")
+        and relative.parts[1].isdigit()
+    ):
+        return Path(*relative.parts[2:])
+    return relative
+
+
+def _collides(exported_paths: list[Path], record_paths: list[Path], destination: Path) -> bool:
+    """Whether dropping the record prefixes would land two different files on
+    one exported path (`service/1/patches/fix.patch` and
+    `service/2/patches/fix.patch` both become `patches/fix.patch`), or a file
+    on the configuration itself; the caller keeps the full record layout then.
+    Case-insensitive: on a case-insensitive filesystem `Fix.patch` and
+    `fix.patch` are one file, and a case-sensitive check would let one copy
+    silently overwrite the other."""
+    record_by_export: dict[str, Path] = {}
+    for record_path, exported_path in zip(record_paths, exported_paths):
+        key = exported_path.as_posix().casefold()
+        if key == destination.name.casefold():
+            return True
+        if record_by_export.setdefault(key, record_path) != record_path:
+            return True
+    return False

src/tests/_internal/cli/services/presets/test_export.py

@@ -32,21 +32,130 @@ def test_exports_a_deployable_service_configuration_with_its_files(self, tmp_pat
             force=False,
         )
 
+        # The store-internal `service/<k>/` record prefix stays out of the
+        # exported layout.
         assert written == [
             destination,
-            tmp_path / "deploy" / "service" / "1" / "patches" / "fix.patch",
+            tmp_path / "deploy" / "patches" / "fix.patch",
         ]
         data = yaml.safe_load(destination.read_text())
         assert data["type"] == "service"
         # Relative to the configuration file, which is how `dstack apply`
         # resolves `files` paths.
+        assert data["files"] == [{"local_path": "patches/fix.patch", "path": "/patches/fix.patch"}]
+        assert (tmp_path / "deploy" / "patches" / "fix.patch").read_text() == "--- a\n+++ b\n"
+        assert ServiceConfiguration.model_validate(data).model is not None
+
+    def test_exports_a_trial_record_file_without_its_record_prefix(self, tmp_path: Path):
+        store = PresetStore(tmp_path / "presets")
+        preset = get_preset()
+        preset.service.files = [
+            FilePathMapping(local_path="trials/3/patches/fix.patch", path="/patches/fix.patch")
+        ]
+        preset_dir = store.save(preset).parent
+        (preset_dir / "trials" / "3" / "patches").mkdir(parents=True)
+        (preset_dir / "trials" / "3" / "patches" / "fix.patch").write_text("--- a\n+++ b\n")
+        destination = tmp_path / "deploy" / "qwen.dstack.yml"
+
+        written = export_preset(
+            store.get(preset.id),
+            preset_dir=preset_dir,
+            destination=destination,
+            force=False,
+        )
+
+        assert written == [destination, tmp_path / "deploy" / "patches" / "fix.patch"]
+        data = yaml.safe_load(destination.read_text())
+        assert data["files"] == [{"local_path": "patches/fix.patch", "path": "/patches/fix.patch"}]
+
+    def test_keeps_record_paths_when_flattening_would_collide(self, tmp_path: Path):
+        store = PresetStore(tmp_path / "presets")
+        preset = get_preset()
+        preset.service.files = [
+            FilePathMapping(local_path="service/1/patches/fix.patch", path="/patches/a.patch"),
+            FilePathMapping(local_path="service/2/patches/fix.patch", path="/patches/b.patch"),
+        ]
+        preset_dir = store.save(preset).parent
+        for attempt, content in (("1", "one\n"), ("2", "two\n")):
+            (preset_dir / "service" / attempt / "patches").mkdir(parents=True)
+            (preset_dir / "service" / attempt / "patches" / "fix.patch").write_text(content)
+        destination = tmp_path / "deploy" / "qwen.dstack.yml"
+
+        written = export_preset(
+            store.get(preset.id),
+            preset_dir=preset_dir,
+            destination=destination,
+            force=False,
+        )
+
+        assert written == [
+            destination,
+            tmp_path / "deploy" / "service" / "1" / "patches" / "fix.patch",
+            tmp_path / "deploy" / "service" / "2" / "patches" / "fix.patch",
+        ]
+        data = yaml.safe_load(destination.read_text())
         assert data["files"] == [
-            {"local_path": "service/1/patches/fix.patch", "path": "/patches/fix.patch"}
+            {"local_path": "service/1/patches/fix.patch", "path": "/patches/a.patch"},
+            {"local_path": "service/2/patches/fix.patch", "path": "/patches/b.patch"},
         ]
         assert (tmp_path / "deploy" / "service" / "1" / "patches" / "fix.patch").read_text() == (
-            "--- a\n+++ b\n"
+            "one\n"
         )
-        assert ServiceConfiguration.model_validate(data).model is not None
+        assert (tmp_path / "deploy" / "service" / "2" / "patches" / "fix.patch").read_text() == (
+            "two\n"
+        )
+
+    def test_treats_paths_differing_only_in_case_as_a_collision(self, tmp_path: Path):
+        store = PresetStore(tmp_path / "presets")
+        preset = get_preset()
+        preset.service.files = [
+            FilePathMapping(local_path="service/1/patches/Fix.patch", path="/patches/a.patch"),
+            FilePathMapping(local_path="service/2/patches/fix.patch", path="/patches/b.patch"),
+        ]
+        preset_dir = store.save(preset).parent
+        for attempt, name in (("1", "Fix.patch"), ("2", "fix.patch")):
+            (preset_dir / "service" / attempt / "patches").mkdir(parents=True)
+            (preset_dir / "service" / attempt / "patches" / name).write_text(name)
+        destination = tmp_path / "deploy" / "qwen.dstack.yml"
+
+        export_preset(
+            store.get(preset.id),
+            preset_dir=preset_dir,
+            destination=destination,
+            force=False,
+        )
+
+        data = yaml.safe_load(destination.read_text())
+        assert data["files"] == [
+            {"local_path": "service/1/patches/Fix.patch", "path": "/patches/a.patch"},
+            {"local_path": "service/2/patches/fix.patch", "path": "/patches/b.patch"},
+        ]
+
+    def test_keeps_record_paths_when_a_file_would_land_on_the_configuration(self, tmp_path: Path):
+        store = PresetStore(tmp_path / "presets")
+        preset = get_preset()
+        preset.service.files = [
+            FilePathMapping(local_path="service/1/qwen.dstack.yml", path="/extra/qwen.dstack.yml")
+        ]
+        preset_dir = store.save(preset).parent
+        (preset_dir / "service" / "1").mkdir(parents=True)
+        (preset_dir / "service" / "1" / "qwen.dstack.yml").write_text("extra\n")
+        destination = tmp_path / "deploy" / "qwen.dstack.yml"
+
+        written = export_preset(
+            store.get(preset.id),
+            preset_dir=preset_dir,
+            destination=destination,
+            force=False,
+        )
+
+        assert written == [destination, tmp_path / "deploy" / "service" / "1" / "qwen.dstack.yml"]
+        data = yaml.safe_load(destination.read_text())
+        assert data["type"] == "service"
+        assert data["files"] == [
+            {"local_path": "service/1/qwen.dstack.yml", "path": "/extra/qwen.dstack.yml"}
+        ]
+        assert (tmp_path / "deploy" / "service" / "1" / "qwen.dstack.yml").read_text() == "extra\n"
 
     def test_refuses_to_overwrite_without_force(self, tmp_path: Path):
         store = PresetStore(tmp_path / "presets")