fix(projects): dedup project_create by primary_path (#75820)
Creating a project whose resolved primary path already belongs to a non-archived project now raises a clear ValueError naming the existing project (create_project) — duplicated projects each seeded an identical copy of the repo subtree, multiplying the duplicate-lane bug per copy. The agent-facing project_create tool is idempotent instead: it re-activates the existing project rather than erroring. allow_duplicate_path=True keeps deliberate duplicates possible. Also updates the legacy non-git lane-id expectation to the branch-style id introduced for #53329.
This commit is contained in:
@@ -319,6 +319,32 @@ def _unique_slug(conn: sqlite3.Connection, candidate: str) -> str:
|
||||
return slug
|
||||
|
||||
|
||||
def _primary_path_key(path: str) -> str:
|
||||
"""Comparison key for primary-path dedup (absolute + case/sep-normalized)."""
|
||||
return os.path.normcase(_normalize_path(path))
|
||||
|
||||
|
||||
def find_by_primary_path(
|
||||
conn: sqlite3.Connection, path: str, *, include_archived: bool = False
|
||||
) -> Optional[Project]:
|
||||
"""The first (oldest) project whose primary path matches ``path``, else None.
|
||||
|
||||
Comparison is separator/case normalized so equivalent Windows spellings of
|
||||
the same folder do not slip past the dedup check.
|
||||
"""
|
||||
key = _primary_path_key(path)
|
||||
if not key:
|
||||
return None
|
||||
for proj in list_projects(conn, include_archived=include_archived):
|
||||
primary = proj.primary_path or next(
|
||||
(f.path for f in proj.folders if f.is_primary),
|
||||
proj.folders[0].path if proj.folders else None,
|
||||
)
|
||||
if primary and _primary_path_key(primary) == key:
|
||||
return proj
|
||||
return None
|
||||
|
||||
|
||||
def create_project(
|
||||
conn: sqlite3.Connection,
|
||||
*,
|
||||
@@ -330,12 +356,19 @@ def create_project(
|
||||
icon: Optional[str] = None,
|
||||
color: Optional[str] = None,
|
||||
board_slug: Optional[str] = None,
|
||||
allow_duplicate_path: bool = False,
|
||||
) -> str:
|
||||
"""Create a project and return its id.
|
||||
|
||||
``folders`` are normalized to absolute paths. If ``primary_path`` is given
|
||||
it is added to the folder set (if not already present) and marked primary;
|
||||
otherwise the first folder becomes primary.
|
||||
|
||||
Duplicate projects pointing at the same folder multiply the sidebar's
|
||||
per-project repo subtrees (every duplicate renders its own copy of the same
|
||||
lanes), so a create whose resolved primary path already belongs to a
|
||||
non-archived project raises ``ValueError`` naming the existing project —
|
||||
pass ``allow_duplicate_path=True`` to bypass deliberately.
|
||||
"""
|
||||
name = str(name or "").strip()
|
||||
if not name:
|
||||
@@ -357,6 +390,14 @@ def create_project(
|
||||
if primary is None and folder_paths:
|
||||
primary = folder_paths[0]
|
||||
|
||||
if primary and not allow_duplicate_path:
|
||||
existing = find_by_primary_path(conn, primary)
|
||||
if existing is not None:
|
||||
raise ValueError(
|
||||
f"folder already belongs to project '{existing.slug}' ({existing.id}); "
|
||||
"switch to it instead of creating a duplicate"
|
||||
)
|
||||
|
||||
with write_txn(conn):
|
||||
unique = _unique_slug(conn, slug_candidate)
|
||||
conn.execute(
|
||||
|
||||
@@ -78,6 +78,49 @@ def test_project_for_path_skips_archived(conn):
|
||||
assert pdb.project_for_path(conn, "/www/app/src").id == pid
|
||||
|
||||
|
||||
def test_create_dedups_by_primary_path(conn):
|
||||
pid = pdb.create_project(conn, name="GeoTrace", folders=["/www/geotrace"])
|
||||
|
||||
# Same folder again (any name): refused, existing project named in error.
|
||||
with pytest.raises(ValueError, match="already belongs to project 'geotrace'"):
|
||||
pdb.create_project(conn, name="GeoTrace", folders=["/www/geotrace"])
|
||||
with pytest.raises(ValueError, match="already belongs"):
|
||||
pdb.create_project(conn, name="Other Name", primary_path="/www/geotrace")
|
||||
|
||||
# Trailing-separator spelling of the same folder is still a duplicate.
|
||||
with pytest.raises(ValueError, match="already belongs"):
|
||||
pdb.create_project(conn, name="GeoTrace", primary_path="/www/geotrace/")
|
||||
|
||||
# Deliberate duplicates stay possible.
|
||||
dup = pdb.create_project(
|
||||
conn, name="GeoTrace", folders=["/www/geotrace"], allow_duplicate_path=True
|
||||
)
|
||||
assert dup != pid
|
||||
assert len(pdb.list_projects(conn)) == 2
|
||||
|
||||
|
||||
def test_create_dedup_ignores_archived_and_other_paths(conn):
|
||||
pid = pdb.create_project(conn, name="App", folders=["/www/app"])
|
||||
pdb.archive_project(conn, pid)
|
||||
|
||||
# Archived project no longer blocks the path.
|
||||
fresh = pdb.create_project(conn, name="App", folders=["/www/app"])
|
||||
assert fresh != pid
|
||||
|
||||
# Different folder is never a collision; folder-less projects don't match.
|
||||
pdb.create_project(conn, name="Elsewhere", folders=["/www/other"])
|
||||
pdb.create_project(conn, name="No Folder")
|
||||
|
||||
|
||||
def test_find_by_primary_path(conn):
|
||||
pid = pdb.create_project(conn, name="App", folders=["/www/app"])
|
||||
|
||||
assert pdb.find_by_primary_path(conn, "/www/app").id == pid
|
||||
assert pdb.find_by_primary_path(conn, "/www/app/").id == pid
|
||||
assert pdb.find_by_primary_path(conn, "/www/nope") is None
|
||||
assert pdb.find_by_primary_path(conn, "") is None
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
@@ -295,7 +295,9 @@ def test_non_git_cwd_preserves_legacy_workspace_grouping():
|
||||
assert project["isAuto"] is True
|
||||
assert project["label"] == "notes"
|
||||
assert project["sessionCount"] == 1
|
||||
assert _lane_ids(project) == ["/work/notes"]
|
||||
# Branch-style lane id (#53329): keying this lane by the raw path used to
|
||||
# fork a duplicate lane against the live overlay's `::branch::main` id.
|
||||
assert _lane_ids(project) == ["/work/notes::branch::main"]
|
||||
assert tree["scoped_session_ids"] == [legacy["id"]]
|
||||
|
||||
|
||||
|
||||
+11
-3
@@ -101,9 +101,17 @@ def project_create(name: str, path: Optional[str] = None, task_id: Optional[str]
|
||||
|
||||
try:
|
||||
with pdb.connect_closing() as conn:
|
||||
pid = pdb.create_project(conn, name=name, folders=[folder] if folder else [], primary_path=folder or None)
|
||||
pdb.set_active(conn, pid)
|
||||
proj = pdb.get_project(conn, pid)
|
||||
existing = pdb.find_by_primary_path(conn, folder) if folder else None
|
||||
if existing is not None:
|
||||
# Idempotent create: the folder already belongs to a project.
|
||||
# Re-activating it beats minting a duplicate — duplicated
|
||||
# projects render N identical sidebar subtrees (#75820).
|
||||
pdb.set_active(conn, existing.id)
|
||||
proj = existing
|
||||
else:
|
||||
pid = pdb.create_project(conn, name=name, folders=[folder] if folder else [], primary_path=folder or None)
|
||||
pdb.set_active(conn, pid)
|
||||
proj = pdb.get_project(conn, pid)
|
||||
except ValueError as exc:
|
||||
return json.dumps({"success": False, "error": str(exc)})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user