fix(projects): scope project path uniqueness per SSH connection (#2731)#2747
fix(projects): scope project path uniqueness per SSH connection (#2731)#2747fiorelorenzo wants to merge 2 commits into
Conversation
Greptile SummaryThis PR fixes a
Confidence Score: 5/5Safe to merge — the migration only loosens a constraint, no existing data can violate the new indexes, and the query fix is logically complete. The two-partial-index approach correctly handles SQLite's NULL distinctness semantics. The migration is non-destructive. The previously flagged orphaned-SSH edge case in getLocalProjectByPath is now properly handled by the added workspaceProvider = 'local' predicate alongside the isNull guard. Tests cover all boundary conditions including the original bug, same-connection rejection, local uniqueness, and local-vs-SSH path sharing. No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/emdash-desktop/drizzle/0019_project_path_unique_per_connection.sql | New migration: drops global idx_projects_path and creates two correctly scoped partial unique indexes; non-destructive and well-formed. |
| apps/emdash-desktop/src/main/db/schema.ts | Replaces single global uniqueIndex with two partial unique indexes matching the migration; Drizzle ORM definitions are correct and well-commented. |
| apps/emdash-desktop/src/main/core/projects/operations/getProjects.ts | getLocalProjectByPath now uses both isNull(sshConnectionId) and eq(workspaceProvider, 'local') to correctly scope local-project lookups and guard the orphaned-SSH edge case. |
| apps/emdash-desktop/src/main/db/tests/migrations/0019_project_path_unique_per_connection.test.ts | Comprehensive migration tests covering index presence, the fixed bug, and all boundary conditions for local and SSH project path uniqueness. |
| apps/emdash-desktop/drizzle/meta/_journal.json | Journal entry for migration 0019 correctly appended with proper idx, version, and tag fields. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[INSERT into projects] --> B{ssh_connection_id IS NULL?}
B -- Yes / Local project --> C[idx_projects_local_path\nUNIQUE path\nWHERE ssh_connection_id IS NULL]
B -- No / SSH project --> D[idx_projects_ssh_connection_path\nUNIQUE ssh_connection_id + path\nWHERE ssh_connection_id IS NOT NULL]
C --> E{Duplicate local path?}
D --> F{Duplicate connection+path?}
E -- Yes --> G[UNIQUE constraint error]
E -- No --> H[Insert OK]
F -- Yes --> G
F -- No --> H
subgraph getLocalProjectByPath
I[path lookup] --> J[AND isNull sshConnectionId\nAND workspaceProvider = 'local']
J --> K{Row found?}
K -- No --> L[undefined]
K -- Yes --> M[LocalProject]
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[INSERT into projects] --> B{ssh_connection_id IS NULL?}
B -- Yes / Local project --> C[idx_projects_local_path\nUNIQUE path\nWHERE ssh_connection_id IS NULL]
B -- No / SSH project --> D[idx_projects_ssh_connection_path\nUNIQUE ssh_connection_id + path\nWHERE ssh_connection_id IS NOT NULL]
C --> E{Duplicate local path?}
D --> F{Duplicate connection+path?}
E -- Yes --> G[UNIQUE constraint error]
E -- No --> H[Insert OK]
F -- Yes --> G
F -- No --> H
subgraph getLocalProjectByPath
I[path lookup] --> J[AND isNull sshConnectionId\nAND workspaceProvider = 'local']
J --> K{Row found?}
K -- No --> L[undefined]
K -- Yes --> M[LocalProject]
end
Reviews (2): Last reviewed commit: "fix(projects): match workspaceProvider i..." | Re-trigger Greptile
…ralaction#2731) `projects.path` had a global UNIQUE index, so registering a project at a path already used by a project on a different SSH host failed with `UNIQUE constraint failed: projects.path`. Paths are independent across separate remote hosts, so uniqueness should be per machine. Replace the global unique index with two partial unique indexes: - local projects (ssh_connection_id IS NULL): unique by path - SSH projects (ssh_connection_id IS NOT NULL): unique by (ssh_connection_id, path) Two indexes rather than a single (ssh_connection_id, path) because SQLite treats NULLs as distinct, which would drop path uniqueness for local projects. As a side effect a local and a remote project may now share a path string (different machines), so getLocalProjectByPath is scoped to local rows to keep inspectProjectPath's lookup correct. Adds migration 0019 plus a migration test covering the index shape and the per-connection / per-machine uniqueness behavior.
5c81a08 to
fad7f0a
Compare
Add eq(workspaceProvider, 'local') alongside the isNull(sshConnectionId) filter so an orphaned SSH project (connection deleted via onDelete: 'set null', leaving ssh_connection_id NULL while workspace_provider stays 'ssh') is no longer returned as a LocalProject. Keeps the discriminant consistent with getProjects/getProjectById; isNull is retained so the query still uses the idx_projects_local_path partial index.
|
@greptileai please re-review. I pushed the fix for your P2 finding in 35b4d47: |
What
projects.pathhas a global UNIQUE index, so registering a project at a path that another host already uses fails, even though the two live on different machines. Since remote hosts commonly mount projects at the same conventional path, adding the same path on a second SSH connection blows up withUNIQUE constraint failed: projects.path.Closes #2731.
Fix
Path uniqueness should be per machine, so the global index becomes two partial unique indexes (migration
0019):ssh_connection_id IS NULL): unique bypathssh_connection_id IS NOT NULL): unique by(ssh_connection_id, path)Two partial indexes rather than a single
(ssh_connection_id, path)one, because SQLite treatsNULLs as distinct: a plain composite index would silently drop path uniqueness for local projects (any number of local projects could share a path). The two-index split is the same partial-index pattern already used foridx_workspaces_keyin this schema.The migration only loosens the constraint (the old global unique already forbade every case the new indexes forbid), so no existing database can violate it and there is nothing to backfill.
Follow-on
Allowing a local and a remote project to share a path string (they are different machines) means a path-only lookup can now match across kinds, so
getLocalProjectByPathis scoped to local rows (ssh_connection_id IS NULL). This keepsinspectProjectPath's local existence check from returning an SSH project.getSshProjectByPathwas already scoped byconnectionId, so it needed no change.Tests
0019_project_path_unique_per_connection.test.tscovers:idx_projects_pathis droppedWHEREclauseformat and lint are clean. Followed the migration authoring flow:
db:generate, committed thepre-0019fixture, regenerated fixtures withdb:fixtures.