Skip to content

fix(projects): prevent duplicate folders and preserve project data - #764

Open
SpookySandwich wants to merge 5 commits into
Porabuild:masterfrom
SpookySandwich:poracode/fair-viper-d98f647d
Open

SpookySandwich wants to merge 5 commits into
Porabuild:masterfrom
SpookySandwich:poracode/fair-viper-d98f647d

Conversation

@SpookySandwich

@SpookySandwich SpookySandwich commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Opening the same folder again now reuses its existing project. Different folders can still share a name, and projects on different machines stay separate.

Reopening a project keeps its custom setup script, actions, and settings. An explicit name or workspace selection is applied to the existing project. Desktop, remote, and MCP creation use the same identity rule, and moving a project to an already registered folder is rejected.

Motivation

Each folder used to receive a new project ID whenever it was added. Preventing new duplicates also needs a safe upgrade for profiles that already have them.

The new database migration moves thread ownership before deleting duplicate rows, preserving conversation history. It also merges notes and to-do lists and repairs saved views, pane layouts, and experiments. Live project updates preserve unsent rich text and attachments, including two populated drafts in split panes. Home remains a separate synthetic project.

The repair keeps the oldest project, regardless of its sidebar position. Its non-empty settings win conflicts; missing settings and distinct actions, MCP entries, and search exclusions are recovered from the other copies. The open app receives the repaired projects and conversations together, and queued saves preserve recovered conversations until the app acknowledges them.

If duplicate projects have watches for the same PR, the repair keeps the watch associated with a recorded fix, including its settings and progress. If both watches record different fixes, both conversations survive and automation pauses until the user chooses a mode again.

Path comparisons cover Windows separators and drive roots, UNC paths, macOS casing, and WSL paths. A desktop's casing rules are not applied to a different remote machine.

Testing

  • Type checking and full lint pass.
  • 414 tests across 21 targeted suites pass, covering database upgrades, transcript and notes preservation, PR-watch collisions, desktop/remote/MCP creation, relocation conflicts, project settings, saved views, and live and queued draft handoff. The app's 34-test suite passed separately after hydration timeouts during parallel runs alongside Electron builds.
  • An isolated Electron smoke run passed the selected app, settings, search, IPC, project, and composer checks with 0 console/runtime errors. External provider, microphone, and remote-device integrations used deterministic mocks.
  • All touched files pass formatting. The full format check still reports the existing CLAUDE.md issue.
  • All 12 non-English catalogs have zero missing translations.
  • Two independent reviews covered database/API and renderer/draft behavior. Their additional findings around notes, colliding PR watches, and queued drafts were fixed and covered by regression tests.
  • GPT-6 Astra adversarially reviewed the settings repair and desktop reconciliation. Its findings were fixed, and its final check passed the earlier queued-save transcript-loss reproducer and deletion cases with no remaining blockers in that scope.
  • The isolated test instance was stopped after verification; its artifacts are retained for inspection.

Screenshots

Multiple projects remain available in the project switcher:

Multiple projects

Reopening the same folder shows an “already registered” notice and keeps one Kurdish-to-English project. This check supplied a fixture folder to the production registration action, with changed casing, separators, and a trailing slash; the project IDs stayed unchanged in memory and in SQLite:

Duplicate creation blocked

@SDSLeon SDSLeon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep review — Request changes

Scope: PR #764 @ 693a44c, 7 files (5 source + 2 screenshots). Verdict: 6 Important — must fix before merge.

Must fix

  • src/renderer/state/appStore.ts:60 — deduping on hydrate emits a project deletion against the pre-merge snapshot; dbSyncChanges deletes the project row before upserting the rehomed threads, and ON DELETE CASCADE destroys those threads' runtime items. Threads survive with empty transcripts, irreversibly, for exactly the users this PR targets.
  • src/renderer/app.tsx:455 — the broadcast path remaps threads only: the current view, draft panes, draft contents and experiments keep pointing at the dropped id, so the user is bounced to Home, loses unsent draft text, and loses experiment grouping.
  • src/renderer/state/slices/projectSlice.ts:88 — the early return drops nameOverride/workspaceId and gives callers no create-vs-reuse signal: re-adding a folder wipes its custom setup script and actions, and re-adding from another workspace is a silent no-op.
  • src/shared/projectIdentity.ts:15 — identity is enforced only in the renderer; projectCommands.register() (MCP create_project, mobile/remote clients) still inserts duplicates that the renderer then deletes, so create_project returns an id that stops existing. Headless backend hosts never dedupe at all.
  • src/shared/projectIdentity.ts:27 — the synthetic Home row carries the home directory as its location and has no exemption, so a project at ~ deduplicates Home away (or gets swallowed by it), orphaning every Home thread.
  • src/shared/projectIdentity.ts:38-45normalizePath collapses the UNC // prefix (false merge of two distinct folders) and, because kind follows the host platform, leaves macOS posix paths case-sensitive — the duplicate this PR targets still occurs on macOS.

Verified good

  • The identity concept is right: location, not name, as the identity boundary, with remoteServerId included so equal paths on different hosts stay distinct — and remoteProjection.ts:50-58 does set both remoteServerId fields synchronously, so remote projects do not collapse.
  • Remote mirror rows are excluded from desktop persistence (appStorePersistence.ts:14), so the dedupe cannot delete a mirrored remote project.
  • merge correctly remaps view via remapProjectView and rewrites thread.projectId — the hydration path's in-memory result is consistent (the gaps are at the DB boundary and in the sibling broadcast path).
  • Targeted suites pass on the PR tree: vitest run src/renderer/state/appStore.test.ts src/shared/projectIdentity.test.ts → 94 passed; tsc --noEmit → 0 errors.
  • src/shared is the right home for the module — it is consumed by both renderer and main, with no provider branching.

Rejected after validation

  • Persist version not bumped to 6 for the dedupe repair: merge already performs unconditional repair-on-read (normalizeStoredThreadStatus) and the on-disk shape is unchanged, and the PR adds a hydration regression test from the old shape. Moving the repair into migrate is still the better shape and is recommended in the appStore.ts:60 comment, but it is not a standalone blocker.
  • projectIdentityKey reads project.remoteServerId rather than location.remoteServerId: both fields are written together at every current write site; no live divergence exists.

Nit-level notes omitted by policy — Important only.

Comment thread src/renderer/state/appStore.ts Outdated
Comment thread src/renderer/app.tsx Outdated
Comment thread src/renderer/state/slices/projectSlice.ts Outdated
Comment thread src/shared/projectIdentity.ts Outdated
Comment thread src/shared/projectIdentity.ts Outdated
Comment thread src/shared/projectIdentity.ts Outdated
@SpookySandwich SpookySandwich changed the title fix(projects): prevent duplicate project locations fix(projects): prevent duplicate folders and preserve project data Sep 13, 2026
@SpookySandwich

SpookySandwich commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

@SDSLeon, this follow-up is ready for another look.

Addressed all six findings in 0427baf:

  1. Duplicate repair now runs in database migration v42, before desktop or headless hydration. Full-state sync also moves dependent rows before deleting a duplicate. Regression tests preserve transcripts, notes, schedules, and PR watches.
  2. Hydration and project broadcasts share the reference-remapping helpers. Views, split draft panes, saved layouts, experiments, unsent rich text, attachments, and queued draft insertions survive, including broadcasts that contain only the surviving project.
  3. Creation reports whether it added or reused a project. Reuse preserves custom scripts/actions, applies an explicit name and workspace selection, skips setup detection and creation analytics, and shows a translated notice.
  4. Main-process registration reuses the existing ID for desktop, remote/mobile, and MCP callers. Relocation rejects an already registered destination.
  5. Home has a separate synthetic identity, so registering the home folder keeps both projects and their threads.
  6. Path identity preserves UNC and drive-root distinctions and applies the host's macOS casing policy without applying it to remote Linux paths.

Two independent reviews also found collisions in notes, PR watches, and pending composer seeds. Those are fixed and covered by regressions. When two merged project records have different recorded PR fixes, both conversations remain and automation pauses until a mode is selected again.

Validation: 394 tests across 17 targeted suites pass, along with type checking and full lint. The final isolated Electron smoke run passed its automated scenarios and 10 mocked integration gates with zero console/runtime errors. All touched files pass formatting; the full formatter still reports the unchanged CLAUDE.md issue. The PR description now includes refreshed screenshots pinned to this commit. The test instance is stopped.

The new CI run is currently marked action_required with no jobs started.

@SDSLeon SDSLeon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep review — Request changes

Scope: PR #764 @ 6cec306 (fix commit 0427bafc + merge of master), 55 files. Verdict: 1 Important — must fix before merge.

Must fix

  • src/main/db/projectDeduplication.ts:13 — v42 picks canonical by sort_order (newest-first from addProject prepend) and deletes the other project row without merging scripts / MCP / search / git-account fields. Upgrade of the exact duplicate population this PR targets drops the original project's custom settings while rehoming threads onto the empty re-add. See inline.

Verified good

  • Hydrate/sync cascade (#1): v42 + rehomeProjectReferences before DELETE, dbSyncAll rehomes before dropping missing project ids; thread_runtime_items asserted in migrations.test.ts and sync.test.ts.
  • Live remaps (#2): applyProjectStateSnapshot rewrites view, draft panes, mounted composer contents, experiments (remap then reconcile), and panel contexts.
  • Reuse (#3): addProjectWithResult returns created, applies name/workspace, gates autoDetectSetupScript / project.added, toasts the localized already-registered message.
  • Authoritative identity (#4): register() reuses by projectIdentityKey; relocate 409s on conflict; MCP returns created: false.
  • Home (#5): projectIdentityKey short-circuits HOME_PROJECT_ID; Home is never a duplicate or canonical target.
  • Path keys (#6): UNC prefix preserved, C:C:\, macOS POSIX opt-in, remotes do not inherit the client's case policy, WSL distro fold + Linux path case.

Rejected after validation

  • Prior 6 Important findings on 693a44c — all fixed on 0427bafc.
  • Optional remote created without a protocol bump — additive field; local MCP always sets it.

Nit-level notes omitted by policy — Important only.

Comment on lines +13 to +20
.prepare("SELECT * FROM projects ORDER BY sort_order ASC, rowid ASC")
.all() as ProjectRow[];
const { duplicateIds } = dedupeProjects(rows.map(rowToProject), {
caseInsensitivePosix: process.platform === "darwin",
});
rehomeProjectReferences(sqlite, duplicateIds);
const deleteProject = sqlite.prepare("DELETE FROM projects WHERE id = ?");
for (const duplicateId of duplicateIds.keys()) deleteProject.run(duplicateId);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important — data loss — migration keeps the newest duplicate and deletes the original project's settings.

What's wrong: repairDuplicateProjects chooses canonical as the first row in ORDER BY sort_order ASC (src/main/db/projectDeduplication.ts:13-17), then deletes every other row (:19-20) after rehoming threads/notes/watches. It never copies scripts, mcp_servers, search_settings, gh_account, worktree_location, icon, or last_draft_config onto the survivor (notes are merged; project-row fields are not).

addProject prepends (projectSlice.ts:125), and dbSyncAll writes sort_order from array index. A user who re-added /repo therefore has: newest nearly-empty duplicate at sort 0 (auto-detected setup script, actions: []) and the original configured project at sort ≥1 (custom setup script and actions, most threads). v42 makes the empty new row canonical and drops the original row. Threads survive on the new id; custom actions/MCP/search/git-account settings do not.

Concrete: P_old created 2024 with 4 actions + 200 threads, P_new created 2026 by re-adding the folder, sort_order 0. First launch after this change → chats reappear under P_new with an empty Actions list, permanently.

Why guards don't stop it: mergeNotes only covers project_notes. Tests seed the keeper at sort 0 (migrations.test.ts:126-127), which is the opposite of production prepend order.

Fix: within each identity group, pick canonical by earliest created_at (stable original), and before delete copy non-empty project settings from discarded rows onto canonical (same idea as mergeNotes). Do not use sidebar sort_order as the keeper.

Regression check: seed two projects at /repo — older row with custom scripts.actions and 3 runtime items, newer row with sort_order = 0 and default/empty scripts; run v42; assert the surviving row still has the custom actions and the 3 runtime items.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 16d835a. The repair now keeps the earliest-created project, regardless of sidebar order, and merges settings before deleting the other rows. The original project's non-empty choices win conflicts; missing settings, distinct actions/MCP entries, and search exclusions are recovered from the other copies. Explicit false values are preserved, and account/model/worktree configurations stay together.

The upgrade regression now uses production ordering: a newer default project at sort 0, an older project with four custom actions at sort 1, and three saved runtime items. It verifies the original ID, actions, recovered settings, and all three messages survive. Hydration and full-state sync use the same repair policy.

GPT-6 Astra's adversarial review also caught a desktop reconciliation gap and a queued-save race. Main now sends repaired projects and recovered conversations back to the desktop together, and protects recovered conversations until a later save acknowledges them. Its final check passed the original race reproducer and project-deletion cases, with no remaining blockers in that scope.

Validation: 414 targeted tests across 21 suites passed across the final runs; the app's 34-test suite passed separately after hydration timeouts while Electron was building. Type checking and full lint pass. The final isolated Electron smoke run passed its automated checks and five mocked integration gates with zero console/runtime errors, then shut down cleanly. All touched files pass formatting; the unchanged CLAUDE.md remains the only full-format warning. Ready for another look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants