fix: close project before deleting project row

[jschwxrz]

No description provided.

[greptile-apps]

Greptile Summary

Fixes a race condition where projectManager.closeProject was called after the database row had already been deleted, meaning the provider teardown could race against a stale record. The fix moves the closeProject call to before the db.delete, inside the mounted-project guard, and adds a new test file that uses Vitest's invocationCallOrder to assert the ordering is correct.

• deleteProject.ts: closeProject is now awaited before prSyncEngine.deleteProjectData and the db.delete, ensuring the provider is fully torn down while the DB row still exists.
• deleteProject.test.ts: New test verifying closeProject precedes the DB delete via invocation-order comparison; only the mounted-project happy path is covered.

Confidence Score: 4/5

The reordering is safe to merge; closeProject wraps all teardown errors in a Result type so it will not throw and block the DB delete in normal operation.

The core logic change is small and clearly correct. The new test validates the ordering for the mounted-project path, but leaves the unmounted path and error-return paths untested, so a future refactor could break those branches silently.

src/main/core/projects/operations/deleteProject.test.ts — the unmounted-project and closeProject-failure test cases are missing.

Important Files Changed

Filename	Overview
src/main/core/projects/operations/deleteProject.ts	Moves closeProject call from after the DB delete to before it, inside the mounted-project guard; the Result return value is still ignored (intentional).
src/main/core/projects/operations/deleteProject.test.ts	New test file that verifies closeProject is called before the DB delete using invocation-order comparison; only covers the mounted-project happy path.

Sequence Diagram

sequenceDiagram
    participant C as Caller
    participant D as deleteProject()
    participant PM as projectManager
    participant TM as taskManager
    participant VS as viewStateService
    participant PR as prSyncEngine
    participant DB as Database

    C->>D: deleteProject(id)
    D->>PM: getProject(id)
    alt project is mounted
        PM-->>D: provider (truthy)
        D->>TM: teardownTask(t.id) [per task]
        D->>VS: "del(task:{id}) [per task]"
        Note over D: await Promise.allSettled(...)
        D->>PM: closeProject(id) ← moved here
        PM-->>D: "Result<void>"
    else project not mounted
        PM-->>D: undefined
    end
    D->>PR: deleteProjectData(id)
    D->>DB: "DELETE projects WHERE id = ?"
    D->>VS: "del(project:{id}) [fire-and-forget]"
    D->>C: emit project:deleted
    D->>C: capture telemetry

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/main/core/projects/operations/deleteProject.test.ts:74-79
**Missing test for unmounted project path**

The suite only exercises the branch where `getProject` returns a truthy provider. There is no test verifying that when `getProject` returns `undefined` the function still deletes the DB row and that `closeProject` is never called. A regression in the `if (provider)` guard (e.g. the call being hoisted out of the block by accident) would go undetected.

### Issue 2 of 2
src/main/core/projects/operations/deleteProject.test.ts:76-79
**Inconsistent non-null assertion on `invocationCallOrder`**

`mocks.closeProject.mock.invocationCallOrder[0]` is used without `!` while `mocks.deleteWhere.mock.invocationCallOrder[0]!` has one. TypeScript types both as `number | undefined`, so only one side is guarded. The `toBeLessThan` call would produce a confusing `undefined < number` failure message rather than a clear assertion error if `closeProject` were never called. Adding `!` (or a preceding `toBeDefined()` assertion) on the left side makes the intent explicit and symmetric.

_{Reviews (1): Last reviewed commit: "fix: close project before deleting proje..." | Re-trigger Greptile}