fix(interface): track allocated subdirs separately from base-name counts

Storing the derived suffix back into name_counts poisoned the counter of any
later target whose base name equalled that suffix, producing surprising (though
still unique) workspace names. Use a dedicated allocated_subdirs set for the
collision guard. Adds a regression test. Addresses Greptile review.
This commit is contained in:
mitnick 2026-06-27 00:13:08 +00:00 committed by Rome-1
parent b2143ef3d9
commit dfecf0343d
2 changed files with 24 additions and 2 deletions

View file

@ -1163,6 +1163,7 @@ def derive_local_base_name(path_str: str) -> str:
def assign_workspace_subdirs(targets_info: list[dict[str, Any]]) -> None:
name_counts: dict[str, int] = {}
allocated_subdirs: set[str] = set()
for target in targets_info:
target_type = target["type"]
@ -1182,15 +1183,17 @@ def assign_workspace_subdirs(targets_info: list[dict[str, Any]]) -> None:
# Avoid colliding with a subdir already allocated to another target
# (e.g. base names ["api-2", "api", "api"] would otherwise both map to "api-2").
# Track allocated subdirs separately from base-name counts so a derived
# suffix never poisons the count of a later target sharing that name.
max_attempts = len(targets_info) + 1
attempts = 0
while workspace_subdir in name_counts and attempts < max_attempts:
while workspace_subdir in allocated_subdirs and attempts < max_attempts:
count += 1
workspace_subdir = f"{base_name}-{count}"
attempts += 1
name_counts[base_name] = count
name_counts[workspace_subdir] = count
allocated_subdirs.add(workspace_subdir)
details["workspace_subdir"] = workspace_subdir

View file

@ -33,6 +33,25 @@ def test_assign_workspace_subdirs_avoids_suffix_collision() -> None:
assert subdirs[2] not in {"api-2", "api"}
def test_assign_workspace_subdirs_suffix_does_not_poison_later_base() -> None:
# Base names ["api", "api", "api-2"]: the second "api" target allocates the
# subdir "api-2"; the later real "api-2" target must still get a unique,
# non-colliding subdir without the earlier allocation poisoning its counter.
targets = [
_local_target("/srv/api"),
_local_target("/other/api"),
_local_target("/srv/api-2"),
]
assign_workspace_subdirs(targets)
subdirs = [t["details"]["workspace_subdir"] for t in targets]
assert len(set(subdirs)) == 3
assert subdirs[0] == "api"
assert subdirs[1] == "api-2"
assert subdirs[2].startswith("api-2")
def test_classify_diff_entries_copy_is_added_not_modified() -> None:
result = _classify_diff_entries(
[DiffEntry(status="C", path="new.py", old_path="orig.py", similarity=100)]