fix(projects): stop "delete all data" projects from resurrecting after reload#949
fix(projects): stop "delete all data" projects from resurrecting after reload#949wideplain wants to merge 3 commits into
Conversation
…r reload
Hard-deleting a project ("すべてのデータを完全に削除" / force=true) only removed
the .jsonl files recorded in `sessions.jsonl_path`. App-created sessions are
registered by `createAppSession` with `jsonl_path = NULL`, so their transcript
at `~/.claude/projects/<encoded-cwd>/<id>.jsonl` was never deleted. The next
`synchronizeSessions()` (which runs on every `GET /api/projects`) re-discovered
the leftover transcript and recreated the project via
`createSession -> createProjectPath`, so the deletion appeared to do nothing
after a page reload.
Changes:
- project-delete.service: on force delete, remove the whole Claude transcript
directory for the project path (path-encoded like server/index.js), guarded so
a malformed project_path can never delete outside ~/.claude/projects. This
fulfils the route's documented "remove all *.jsonl under the Claude project dir".
- projects.db `createProjectPath`: add `reactivateArchived` (default true). The
background synchronizer now passes false so a passive re-scan can no longer
silently un-archive a project the user archived (the `ON CONFLICT ... SET
isArchived = 0` reactivation previously fired on any transcript re-touch).
- providers barrel: export `ClaudeSessionSynchronizer` for tests.
- tests: regression suite covering indexed + app-created (null jsonl_path)
deletion, out-of-root safety, and archive-not-reactivated-on-reindex.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- deleteClaudeProjectDir: stop swallowing fs.rm errors. `force: true` already ignores a missing directory, so a remaining throw (e.g. permission error) means transcripts survived; propagating it aborts the delete before DB rows are removed, so we never report a "deleted" project that resurrects on reload. - createProjectPath: return new `archived_conflict` outcome (instead of mislabeling as `active_conflict`) when an archived row is left archived under `reactivateArchived: false`. Extracted a named `CreateProjectPathOptions` type. - createProject: treat `archived_conflict` like `active_conflict` (unreachable with default reactivation, handled defensively for sound typing). - tests: add archived_conflict outcome coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds soft-delete support for projects, extends ChangesArchived conflict handling and force-delete transcript cleanup
Sequence Diagram(s)sequenceDiagram
participant Caller
participant projectsDb
participant SQLite
Caller->>projectsDb: createProjectPath(path, name, options)
projectsDb->>SQLite: INSERT ... ON CONFLICT(project_path)
alt reactivateArchived true
SQLite-->>projectsDb: clear isArchived, isDeleted, deleted_at
else reactivateArchived false
SQLite-->>projectsDb: DO NOTHING
end
projectsDb-->>Caller: created / reactivated_archived / active_conflict / archived_conflict
sequenceDiagram
participant deleteOrArchiveProject
participant project-delete.service
participant FileSystem
participant projectsDb
deleteOrArchiveProject->>project-delete.service: force delete project
project-delete.service->>project-delete.service: resolveClaudeProjectDir(project_path)
alt path within Claude projects root
project-delete.service->>FileSystem: fs.rm(dir, recursive, force)
project-delete.service->>projectsDb: markProjectDeletedById(projectId)
else path escapes root
project-delete.service-->>deleteOrArchiveProject: warn and skip directory removal
end
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/modules/projects/tests/project-delete.service.test.ts (1)
85-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis test does not actually exercise the out-of-root guard.
'../../../precious'is passed through the same encoding as production, which turns every separator/dot into-, so the resolved dir is always a child of the root (~/.claude/projects/--------precious) andfs.rmjust no-ops on a missing path. The assertion would still pass even ifresolveClaudeProjectDir'sreturn nullbranch were deleted, so that safety branch is currently untested. Consider adding a case that forcesresolveClaudeProjectDirto returnnull(e.g. asserting on the resolver directly, or an input that yields an empty/.encoding) to genuinely cover the guard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/modules/projects/tests/project-delete.service.test.ts` around lines 85 - 97, The current force-delete test in project-delete.service.test.ts is not reaching the out-of-root protection in resolveClaudeProjectDir, because createProjectPath encodes the malformed path into a safe child directory. Update the test to directly exercise the null-return guard in resolveClaudeProjectDir, or use an input that actually resolves to an empty or dot-like path, and assert that deleteOrArchiveProject does not attempt removal outside the Claude projects root. Keep the focus on the resolver and deleteOrArchiveProject behavior so the safety branch is genuinely covered.server/modules/projects/services/project-delete.service.ts (1)
59-100: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winExtract the Claude project-dir encoding into one shared helper
server/index.js,project-delete.service.ts, and the test all duplicate the sameprojectPath.replace(/[^a-zA-Z0-9-]/g, '-')logic. Move that encoding into a shared helper and reuse it on the write and delete paths so the force-delete behavior can’t drift from the on-disk naming.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/modules/projects/services/project-delete.service.ts` around lines 59 - 100, The Claude project-dir encoding is duplicated across the write path, delete path, and tests, so extract the `projectPath.replace(/[^a-zA-Z0-9-]/g, '-')` logic into one shared helper and reuse it from `resolveClaudeProjectDir` and the corresponding creation/indexing code. Update `deleteClaudeProjectDir` and the code in `server/index.js` to call that helper so the on-disk naming stays consistent and the delete behavior cannot drift from the session directory layout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/modules/projects/services/project-delete.service.ts`:
- Around line 59-100: The Claude project-dir encoding is duplicated across the
write path, delete path, and tests, so extract the
`projectPath.replace(/[^a-zA-Z0-9-]/g, '-')` logic into one shared helper and
reuse it from `resolveClaudeProjectDir` and the corresponding creation/indexing
code. Update `deleteClaudeProjectDir` and the code in `server/index.js` to call
that helper so the on-disk naming stays consistent and the delete behavior
cannot drift from the session directory layout.
In `@server/modules/projects/tests/project-delete.service.test.ts`:
- Around line 85-97: The current force-delete test in
project-delete.service.test.ts is not reaching the out-of-root protection in
resolveClaudeProjectDir, because createProjectPath encodes the malformed path
into a safe child directory. Update the test to directly exercise the
null-return guard in resolveClaudeProjectDir, or use an input that actually
resolves to an empty or dot-like path, and assert that deleteOrArchiveProject
does not attempt removal outside the Claude projects root. Keep the focus on the
resolver and deleteOrArchiveProject behavior so the safety branch is genuinely
covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 38689976-78d8-4e0a-8196-b098757bdb20
📒 Files selected for processing (8)
server/modules/database/repositories/projects.db.tsserver/modules/database/repositories/sessions.db.tsserver/modules/database/tests/projects.db.integration.test.tsserver/modules/projects/services/project-delete.service.tsserver/modules/projects/services/project-management.service.tsserver/modules/projects/tests/project-delete.service.test.tsserver/modules/providers/index.tsserver/shared/types.ts
wideplain
left a comment
wideplain
left a comment
There was a problem hiding this comment.
レビュー結論: Approve 相当（ブロッキングなし）
報告バグ（「すべてのデータを完全に削除」してもリロードでプロジェクトが復活）を正しく修正しており、再現テスト（特に app 発の jsonl_path=NULL ケース）で回帰が担保されています。以下を一次情報で確認済み:
- エンコード整合性:
resolveClaudeProjectDirのreplace(/[^a-zA-Z0-9-]/g,'-')は既存のserver/index.jsの規約と一致し、実機の~/.claude/projects実ディレクトリ名とも一致（/Users/koya→-Users-koya等）。 - アーカイブ非解除ガード:
reactivateArchived:falseは sync 経路（createSession）のみに適用され、createAppSession/createProject/agent 登録はデフォルトtrueのまま。全プロバイダ synchronizer はcreateSession経由なので漏れなし。 - エラー順序:
deleteClaudeProjectDirが throw すると DB 行削除の前に中断し、asyncHandler→グローバルエラーミドルウェアで 500（絶対パスはクライアントに漏れずサーバログのみ）。 archived_conflict:DO NOTHING→行なし→フォールバックで archived を正しく判定。
非ブロッキングのフォローアップ（別対応可）
テスト網羅の隙間（Low） — 以下は現状未カバー。任意で追加を推奨:
fs.rm失敗時に DB 行削除の前に中断する不変条件のテスト（fs.promises.rmを throw させ、getProjectPaths().lengthが減らないことを確認）。- Claude ディレクトリが存在しない（ENOENT）プロジェクトの force 削除が throw しないこと。
- ドット/アンダースコアを含むパス（例
/Users/x/my.app）でのエンコード round-trip。
個別の指摘は該当行のインラインコメントを参照してください。
| return; | ||
| } | ||
|
|
||
| await fs.rm(projectDir, { recursive: true, force: true }); |
There was a problem hiding this comment.
[Medium] 復活防止が Claude 専用スコープ。 deleteClaudeProjectDir は ~/.claude/projects/<encoded> のみ除去しますが、createAppSession は claude 以外でも呼ばれます（provider.routes.ts:539 → sessions.service.ts:133、cursor/codex/gemini/opencode）。非 claude プロバイダの app 発セッション（jsonl_path=NULL）はトランスクリプトが ~/.cursor/projects 等に残り、force 削除後の次回 sync で createProjectPath が新規作成 → 同じ復活バグが非 claude では残存します。
推奨: (a) 関数 JSDoc と PR 説明に「Claude 専用」であることを明記、または (b) sessions-watcher.service.ts の PROVIDER_WATCH_PATHS が各プロバイダの root を持っているので、削除対象プロバイダの transcript dir も除去する、あるいは (c) 削除済み project_path の tombstone を持たせて sync 側で再作成をスキップする。
There was a problem hiding this comment.
対応しました（ff58c88）。プロバイダ非依存の tombstone 方式に変更しました。projects に isDeleted/deleted_at を追加し、force 削除は行を tombstone（session行・追跡jsonl・Claude dir は従来どおり削除）。sessionsDb.createSession（sync/watcher 経路）は tombstone 済みパスの stale なトランスクリプト（activity ≤ deleted_at）からの再作成をスキップし、deleted_at より新しい活動 or 明示的な再作成（createAppSession/createProject）で解除します。これで Cursor/Codex/Gemini/OpenCode でも復活しません。テスト追加済み（stale codex は復活せず／新活動で復活／明示再作成で復活）。
| * conventional lookup in `server/index.js`). Returns `null` when the resolved path would | ||
| * escape the Claude projects root, so a malformed `project_path` can never delete outside it. | ||
| */ | ||
| function resolveClaudeProjectDir(projectPath: string): string | null { |
There was a problem hiding this comment.
[Low] エンコード衝突が新たに破壊的になり得る。 /[^a-zA-Z0-9-]/g→'-' により、/Users/x/foo_bar と /Users/x/foo.bar は同じ -Users-x-foo-bar にエンコードされます。PR 前の削除は sessions.jsonl_path の実パスのみ unlink していた（衝突なし）のに対し、本 PR は encoded ディレクトリごと fs.rm するため、別の（まだ有効な）プロジェクトのトランスクリプトを巻き込んで削除する可能性があります。
（Claude 自身も同じエンコードで保存するため両者は元々ディスク上同一ディレクトリを共有しており影響は限定的ですが、DB 上は別行なので一方の force 削除で他方のディスクデータが消える点が新たな挙動です。）
推奨: fs.rm 前に「同じ encoded 名に解決される有効な project 行が他に無いか」を確認して、あれば削除を拒否する。あるいは作成時に encoded dir 名を保存して一意性を担保する。
There was a problem hiding this comment.
この tombstone 対応では未変更です（別事象）。エンコード衝突（/foo_bar と /foo.bar が同一 encoded dir）は Claude の dir 削除に固有で、Claude 自身も同じエンコードで保存するため元々ディスク上は共有されている点も踏まえ、Low として本 PR では据え置きます。必要なら「同一 encoded 名に解決される有効プロジェクトが他に無いか確認してから rm」を別途入れます。
| // `createProject` uses the default reactivation, so an archived path resolves to | ||
| // `reactivated_archived`; `archived_conflict` is only reachable when a caller opts out. | ||
| // Both conflict outcomes mean an existing row blocked a fresh insert, so treat them alike. | ||
| if (persistedProject.outcome === 'active_conflict' || persistedProject.outcome === 'archived_conflict') { |
There was a problem hiding this comment.
[Nit] このガードは archived_conflict も捕捉しますが、createProject はデフォルト reactivateArchived:true のため archived は reactivated_archived に解決され、archived_conflict はここでは到達不能です（防御的分岐としては妥当）。ただしエラーメッセージ "already exists and is active" は archived ケースに当たると不正確なので、"already exists (active or archived)" 等に緩めると将来の保守者が混乱しません。
…viders via tombstone The Claude-only directory removal (previous commit) does not cover Cursor/Codex/ Gemini/OpenCode: only Claude stores transcripts in a per-cwd directory that can be removed. The others keep them in flat/hashed/shared-DB layouts keyed by the cwd inside each file, so an app-created session whose `jsonl_path` was still NULL can leave a transcript we cannot locate — which the synchronizer then re-imports, recreating the project on reload. Fix with a provider-agnostic tombstone: - projects table gains `isDeleted` + `deleted_at` (schema + migration, both the add-column and table-rebuild paths). - Force delete now tombstones the row (`markProjectDeletedById`) instead of hard deleting it, after removing session rows, tracked jsonl files, and the Claude dir. - `sessionsDb.createSession` (the background sync/watcher path) skips recreating a tombstoned project from a STALE transcript (activity <= deleted_at) and returns ''. Genuinely new activity (after deleted_at) lifts the tombstone and indexes normally. - Explicit user actions (createAppSession / createProject) revive a tombstoned path via createProjectPath's reactivation branch (now also clears isDeleted/deleted_at). - getProjectPaths / getArchivedProjectPaths exclude tombstoned rows. Tests: stale non-claude (Codex) transcript stays deleted; new activity revives; explicit createAppSession revives; existing archive/delete guarantees unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Problem
Deleting a project with "Delete all data" (
force=true) appears to do nothing after a page reload — the "deleted" project comes back. Archiving can also silently reverse itself.Root cause (reproduced with a test)
deleteOrArchiveProject(force=true)→deleteSessionJsonlFilesForProjectPathonly unlinks files recorded insessions.jsonl_path. But sessions started from the app UI are registered bycreateAppSessionwithjsonl_path = NULL, so the real transcript at~/.claude/projects/<encoded-cwd>/<id>.jsonlis never removed.The project list is rebuilt from disk on every
GET /api/projects:synchronizeSessions()→ClaudeSessionSynchronizer.synchronize()re-discovers the leftover transcript →createSession()→createProjectPath()recreates the project. The route's own doc comment says it should "remove all*.jsonlunder the Claude project dir", but the implementation didn't.Separately,
createProjectPath'sON CONFLICT ... SET isArchived = 0reactivates an archived project on any transcript re-touch (e.g. a file-watcherchangeevent), so archiving doesn't reliably stick either.Fix
project-delete.service: on force delete, remove the whole Claude transcript directory for the project path (encoded the same way asserver/index.js), guarded so a malformedproject_pathcan never delete outside~/.claude/projects.fs.rmerrors are not swallowed —force: truealready ignores a missing dir, so a remaining throw means transcripts survived; propagating it aborts the delete before the DB rows are removed, so we never report success (and resurrect) on a failed cleanup.projects.db.createProjectPath: addreactivateArchived(defaulttrue, via a namedCreateProjectPathOptionstype). The background synchronizer passesfalse, so a passive re-scan can no longer un-archive a project the user archived. When an archived row is left archived, the outcome is reported as the newarchived_conflict(instead of mislabeling itactive_conflict).ClaudeSessionSynchronizerfor tests.Tests
New
project-delete.service.test.ts(4 cases) + acreateProjectPatharchived_conflictoutcome test, all green:jsonl_path) transcript so it cannot resurrect ← the reported bugnpm run typecheck, eslint, and the projects/database test suites pass.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes